Subject: Re: [PATCH v2 net-next 1/8] bridge: Turn flag change macro into a function.

From: "Michael S. Tsirkin" <[email-masked]>
Date: Thu, 15 May 2014 20:18:22 +0300

On Thu, May 15, 2014 at 12:56:49PM -0400, Vlad Yasevich wrote:
> Turn the flag change macro into a function to allow
> easier updates and to reduce space.
> 
> Signed-off-by: Vlad Yasevich <[email-masked]>

Acked-by: Michael S. Tsirkin <[email-masked]>

whitespace nit below

> ---
>  net/bridge/br_sysfs_if.c | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
> index dd595bd..112a25e 100644
> --- a/net/bridge/br_sysfs_if.c
> +++ b/net/bridge/br_sysfs_if.c
> @@ -41,20 +41,27 @@ static ssize_t show_##_name(struct net_bridge_port *p, char *buf) \
>  }								\
>  static int store_##_name(struct net_bridge_port *p, unsigned long v) \
>  {								\
> -	unsigned long flags = p->flags;				\
> -	if (v)							\
> -		flags |= _mask;					\
> -	else							\
> -		flags &= ~_mask;				\
> -	if (flags != p->flags) {				\
> -		p->flags = flags;				\
> -		br_ifinfo_notify(RTM_NEWLINK, p);		\
> -	}							\
> -	return 0;						\
> +	return store_flag(p, v, _mask);				\
>  }								\
>  static BRPORT_ATTR(_name, S_IRUGO | S_IWUSR,			\
>  		   show_##_name, store_##_name)
>  
> +static int store_flag(struct net_bridge_port *p, unsigned long v,
> +		     unsigned long mask)

This second line should align "unsigned" with "struct" on the line above
it. As it is it's aligned with (  which violates the docinf style
requirement
	Descendants are always substantially shorter than the parent and
	are placed substantially to the right.
it's not substantially to the right if it's not to the right of (.

> +{
> +	unsigned long flags = p->flags;
> +
> +	if (v)
> +		flags |= mask;
> +	else
> +		flags &= ~mask;
> +
> +	if (flags != p->flags) {
> +		p->flags = flags;
> +		br_ifinfo_notify(RTM_NEWLINK, p);
> +	}
> +	return 0;
> +}
>  
>  static ssize_t show_path_cost(struct net_bridge_port *p, char *buf)
>  {
> -- 
> 1.9.0