Re: [PATCH 1/3] Remove trailing NULs from network bonding sysfs interface.

[Randy Dunlap <randy.dunlap@oracle.com>]

On Wed, 28 Nov 2007 01:49:54 +0100 =?utf-8?q?Ferenc_W=C3=A1gner?= wrote:

> Also remove trailing spaces from multivalued files.
> 
> This fixes output like for example:
> 
> $ od -c /sys/class/net/bond0/bonding/slaves
> 0000000   e   t   h   -   l   e   f   t       e   t   h   -   r   i   g
> 0000020   h   t      \n  \0
> 0000025
> 
> It mostly entails deleting '+1'-s after sprintf() calls: the return value
> of sprintf is the number of characters printed, without the closing NUL,
> ie. exactly what the sysfs interface requires.  The three multivalue
> cases are different, because they also have to swallow back a trailing
> space.
> 
> Signed-off-by: Ferenc Wágner <wferi@niif.hu>
> ---
>  drivers/net/bonding/bond_sysfs.c |   66 +++++++++++++++++--------------------
>  1 files changed, 30 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
> index b29330d..a3f1b4a 100644
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -86,14 +86,13 @@ static ssize_t bonding_show_bonds(struct class *cls, char *buffer)
>  			/* not enough space for another interface name */
>  			if ((PAGE_SIZE - res) > 10)
>  				res = PAGE_SIZE - 10;
> -			res += sprintf(buffer + res, "++more++");
> +			res += sprintf(buffer + res, "++more++ ");
>  			break;
>  		}
>  		res += sprintf(buffer + res, "%s ",
>  			       bond->dev->name);
>  	}
> -	res += sprintf(buffer + res, "\n");
> -	res++;
> +	if (res) buffer[res-1] = '\n'; /* eat the leftover space */
>  	up_read(&(bonding_rwsem));
>  	return res;
>  }
> @@ -235,14 +234,13 @@ static ssize_t bonding_show_slaves(struct device *d,
>  			/* not enough space for another interface name */
>  			if ((PAGE_SIZE - res) > 10)
>  				res = PAGE_SIZE - 10;
> -			res += sprintf(buf + res, "++more++");
> +			res += sprintf(buf + res, "++more++ ");
>  			break;
>  		}
>  		res += sprintf(buf + res, "%s ", slave->dev->name);
>  	}
>  	read_unlock(&bond->lock);
> -	res += sprintf(buf + res, "\n");
> -	res++;
> +	if (res) buf[res-1] = '\n'; /* eat the leftover space */
>  	return res;
>  }
>  
> @@ -711,10 +709,7 @@ static ssize_t bonding_show_arp_targets(struct device *d,
>  			res += sprintf(buf + res, "%u.%u.%u.%u ",
>  			       NIPQUAD(bond->params.arp_targets[i]));
>  	}
> -	if (res)
> -		res--;  /* eat the leftover space */
> -	res += sprintf(buf + res, "\n");
> -	res++;
> +	if (res) buf[res-1] = '\n'; /* eat the leftover space */
>  	return res;
>  }

Hi,

Patches 1 & 3 use

	if (res) statement;

but the preferred form is

	if (res)
		statement;

Even if this style was already used in the source file, it should
be cleaned up.

Thanks,
---
~Randy