Re: [PATCH nf-next] ipvs: change some data types from int to bool

Re: [PATCH nf-next] ipvs: change some data types from int to bool

[Simon Horman]

On Sat, Feb 16, 2019 at 04:39:53PM +0100, Andrea Claudi wrote:
> Change the data type of the following variables from int to bool
> across ipvs code:
> 
>   - found
>   - loop
>   - need_full_dest
>   - need_full_svc
>   - payload_csum
> 
> Also change the following functions to use bool full_entry param
> instead of int:
> 
>   - ip_vs_genl_parse_dest()
>   - ip_vs_genl_parse_service()
> 
> This patch does not change any functionality but makes the source
> code slightly easier to read.
> 
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>


Acked-by: Simon Horman <horms@verge.net.au>

Pablo,

please consider applying this to nf-next.

> ---
>  net/netfilter/ipvs/ip_vs_ctl.c       | 12 ++++++------
>  net/netfilter/ipvs/ip_vs_ftp.c       |  4 ++--
>  net/netfilter/ipvs/ip_vs_proto_tcp.c |  8 ++++----
>  net/netfilter/ipvs/ip_vs_proto_udp.c |  8 ++++----
>  net/netfilter/ipvs/ip_vs_xmit.c      |  4 ++--
>  5 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index ac8d848d7624..0665ffca407f 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3088,7 +3088,7 @@ static bool ip_vs_is_af_valid(int af)
>  
>  static int ip_vs_genl_parse_service(struct netns_ipvs *ipvs,
>  				    struct ip_vs_service_user_kern *usvc,
> -				    struct nlattr *nla, int full_entry,
> +				    struct nlattr *nla, bool full_entry,
>  				    struct ip_vs_service **ret_svc)
>  {
>  	struct nlattr *attrs[IPVS_SVC_ATTR_MAX + 1];
> @@ -3175,7 +3175,7 @@ static struct ip_vs_service *ip_vs_genl_find_service(struct netns_ipvs *ipvs,
>  	struct ip_vs_service *svc;
>  	int ret;
>  
> -	ret = ip_vs_genl_parse_service(ipvs, &usvc, nla, 0, &svc);
> +	ret = ip_vs_genl_parse_service(ipvs, &usvc, nla, false, &svc);
>  	return ret ? ERR_PTR(ret) : svc;
>  }
>  
> @@ -3285,7 +3285,7 @@ static int ip_vs_genl_dump_dests(struct sk_buff *skb,
>  }
>  
>  static int ip_vs_genl_parse_dest(struct ip_vs_dest_user_kern *udest,
> -				 struct nlattr *nla, int full_entry)
> +				 struct nlattr *nla, bool full_entry)
>  {
>  	struct nlattr *attrs[IPVS_DEST_ATTR_MAX + 1];
>  	struct nlattr *nla_addr, *nla_port;
> @@ -3547,11 +3547,11 @@ static int ip_vs_genl_set_daemon(struct sk_buff *skb, struct genl_info *info)
>  
>  static int ip_vs_genl_set_cmd(struct sk_buff *skb, struct genl_info *info)
>  {
> +	bool need_full_svc = false, need_full_dest = false;
>  	struct ip_vs_service *svc = NULL;
>  	struct ip_vs_service_user_kern usvc;
>  	struct ip_vs_dest_user_kern udest;
>  	int ret = 0, cmd;
> -	int need_full_svc = 0, need_full_dest = 0;
>  	struct net *net = sock_net(skb->sk);
>  	struct netns_ipvs *ipvs = net_ipvs(net);
>  
> @@ -3575,7 +3575,7 @@ static int ip_vs_genl_set_cmd(struct sk_buff *skb, struct genl_info *info)
>  	 * received a valid one. We need a full service specification when
>  	 * adding / editing a service. Only identifying members otherwise. */
>  	if (cmd == IPVS_CMD_NEW_SERVICE || cmd == IPVS_CMD_SET_SERVICE)
> -		need_full_svc = 1;
> +		need_full_svc = true;
>  
>  	ret = ip_vs_genl_parse_service(ipvs, &usvc,
>  				       info->attrs[IPVS_CMD_ATTR_SERVICE],
> @@ -3595,7 +3595,7 @@ static int ip_vs_genl_set_cmd(struct sk_buff *skb, struct genl_info *info)
>  	if (cmd == IPVS_CMD_NEW_DEST || cmd == IPVS_CMD_SET_DEST ||
>  	    cmd == IPVS_CMD_DEL_DEST) {
>  		if (cmd != IPVS_CMD_DEL_DEST)
> -			need_full_dest = 1;
> +			need_full_dest = true;
>  
>  		ret = ip_vs_genl_parse_dest(&udest,
>  					    info->attrs[IPVS_CMD_ATTR_DEST],
> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> index 4398a72edec5..fe69d46ff779 100644
> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> @@ -124,7 +124,7 @@ static int ip_vs_ftp_get_addrport(char *data, char *data_limit,
>  	}
>  	s = data + plen;
>  	if (skip) {
> -		int found = 0;
> +		bool found = false;
>  
>  		for (;; s++) {
>  			if (s == data_limit)
> @@ -136,7 +136,7 @@ static int ip_vs_ftp_get_addrport(char *data, char *data_limit,
>  				if (!ext && isdigit(*s))
>  					break;
>  				if (*s == skip)
> -					found = 1;
> +					found = true;
>  			} else if (*s != skip) {
>  				break;
>  			}
> diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> index 1770fc6ce960..a95ef1d77159 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> @@ -149,8 +149,8 @@ tcp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
>  {
>  	struct tcphdr *tcph;
>  	unsigned int tcphoff = iph->len;
> +	bool payload_csum = false;
>  	int oldlen;
> -	int payload_csum = 0;
>  
>  #ifdef CONFIG_IP_VS_IPV6
>  	if (cp->af == AF_INET6 && iph->fragoffs)
> @@ -176,7 +176,7 @@ tcp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
>  		if (ret == 1)
>  			oldlen = skb->len - tcphoff;
>  		else
> -			payload_csum = 1;
> +			payload_csum = true;
>  	}
>  
>  	tcph = (void *)skb_network_header(skb) + tcphoff;
> @@ -227,8 +227,8 @@ tcp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
>  {
>  	struct tcphdr *tcph;
>  	unsigned int tcphoff = iph->len;
> +	bool payload_csum = false;
>  	int oldlen;
> -	int payload_csum = 0;
>  
>  #ifdef CONFIG_IP_VS_IPV6
>  	if (cp->af == AF_INET6 && iph->fragoffs)
> @@ -257,7 +257,7 @@ tcp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
>  		if (ret == 1)
>  			oldlen = skb->len - tcphoff;
>  		else
> -			payload_csum = 1;
> +			payload_csum = true;
>  	}
>  
>  	tcph = (void *)skb_network_header(skb) + tcphoff;
> diff --git a/net/netfilter/ipvs/ip_vs_proto_udp.c b/net/netfilter/ipvs/ip_vs_proto_udp.c
> index 0f53c49025f8..d914e077d51b 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_udp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_udp.c
> @@ -139,8 +139,8 @@ udp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
>  {
>  	struct udphdr *udph;
>  	unsigned int udphoff = iph->len;
> +	bool payload_csum = false;
>  	int oldlen;
> -	int payload_csum = 0;
>  
>  #ifdef CONFIG_IP_VS_IPV6
>  	if (cp->af == AF_INET6 && iph->fragoffs)
> @@ -168,7 +168,7 @@ udp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
>  		if (ret == 1)
>  			oldlen = skb->len - udphoff;
>  		else
> -			payload_csum = 1;
> +			payload_csum = true;
>  	}
>  
>  	udph = (void *)skb_network_header(skb) + udphoff;
> @@ -222,8 +222,8 @@ udp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
>  {
>  	struct udphdr *udph;
>  	unsigned int udphoff = iph->len;
> +	bool payload_csum = false;
>  	int oldlen;
> -	int payload_csum = 0;
>  
>  #ifdef CONFIG_IP_VS_IPV6
>  	if (cp->af == AF_INET6 && iph->fragoffs)
> @@ -252,7 +252,7 @@ udp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
>  		if (ret == 1)
>  			oldlen = skb->len - udphoff;
>  		else
> -			payload_csum = 1;
> +			payload_csum = true;
>  	}
>  
>  	udph = (void *)skb_network_header(skb) + udphoff;
> diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
> index 473cce2a5231..175349fcf91f 100644
> --- a/net/netfilter/ipvs/ip_vs_xmit.c
> +++ b/net/netfilter/ipvs/ip_vs_xmit.c
> @@ -126,7 +126,7 @@ static struct rtable *do_output_route4(struct net *net, __be32 daddr,
>  {
>  	struct flowi4 fl4;
>  	struct rtable *rt;
> -	int loop = 0;
> +	bool loop = false;
>  
>  	memset(&fl4, 0, sizeof(fl4));
>  	fl4.daddr = daddr;
> @@ -149,7 +149,7 @@ static struct rtable *do_output_route4(struct net *net, __be32 daddr,
>  		ip_rt_put(rt);
>  		*saddr = fl4.saddr;
>  		flowi4_update_output(&fl4, 0, 0, daddr, fl4.saddr);
> -		loop++;
> +		loop = true;
>  		goto retry;
>  	}
>  	*saddr = fl4.saddr;
> -- 
> 2.20.1
> 

Re: [PATCH nf-next] ipvs: change some data types from int to bool

[Pablo Neira Ayuso]

On Sat, Feb 16, 2019 at 04:39:53PM +0100, Andrea Claudi wrote:
> Change the data type of the following variables from int to bool
> across ipvs code:
> 
>   - found
>   - loop
>   - need_full_dest
>   - need_full_svc
>   - payload_csum
> 
> Also change the following functions to use bool full_entry param
> instead of int:
> 
>   - ip_vs_genl_parse_dest()
>   - ip_vs_genl_parse_service()
> 
> This patch does not change any functionality but makes the source
> code slightly easier to read.

Applied, thanks.