Message-ID: <50ADE447.8030300@asianux.com>
Date:	Thu, 22 Nov 2012 16:37:27 +0800
From:	Chen Gang <gang.chen@...anux.com>
To:	Shan Wei <shanwei88@...il.com>
CC:	Eric Dumazet <eric.dumazet@...il.com>,
	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: ipv6: change %8s to %s for rt->dst.dev->name in
 seq_printf of rt6_info_route

于 2012年11月22日 13:28, Shan Wei 写道:
> Hi chen gang:
> 
> For length of device name which less than 8 char，
> your patch changes them to be print from align right 
> to align left. But at least since 2005(git age-time),
> we keep this style so far.
> Maybe, since birth of this code, just align right. :-)
> 

  originally, it is a solid output length, the length is "#define
RT6_INFO_LEN (32 + 4 + 32 + 4 + 32 + 40 + 5 + 1)"
  and RHEL5 (kernel-2.6.18-308.20.el5) still use it.
  it assume that the length of rt->rt6i_dev->name (in RHEL5) is 8.

> Why we *should* change this style?
> just keep be consistent with the case which length of device
> name greater than 8 char?
> 

  as a solid length, 8 is not suitable, firstly I suggest to '%16s' (I
call it 'beautiful',  but for RHEL5, it is a correctness issue)
  and Eric Dumazet suggest use '%s' is better, since it is not solid
length any more (have already let seq_printf instead of arg->buffer)
  and I think: as a result, what he said is reasonable

> Not only old name rule i.e. eth0,eth1, but also new name rule
> base on pci address ,i.e. em1,p3p1. most of them are less than 8 char.
> Should not we take more attention on the case less than 8 char?
> 

  I have ever seen such a device name is more than 8 characters.
  I am not quite sure: maybe they are eth-route* or eth-usb* ...
  I will check it in these days, please wait for some days.


> By addition, if we want to add new field in the future,
> align right is a better choice.
> 

  maybe what you said is better (still keep it 'beautiful', but need use
'%16s' instead of '%8s')

  for this, Eric Dumazet maybe have his opinions.


 Regards

gchen.

> 
> Chen Gang said, at 2012/11/22 10:52:
>> Hi Shan Wei, Eric Dumazet
>>
>>   is this patch integrated into main branch ?
>>   if need me for additional completion (such as: merge another 2 trivial patches into this patch, too)
>>   please tell me, I will do. 
>>
>>   I understand you are working overtime, maybe no time for any minor and trivial patches.
>>   if surely it is, I think:
>>     you can modify these code manually, and obsolete these minor and trivial patches which I provided.
>>     I do not mind whether mention me in another new patches (you can mention me or not mention me, both are OK).
>>     since our goal is to provide contributes to outside, efficiently.
>>
>>  regards
>>
>> gchen
>>
>>
>> 于 2012年11月05日 11:02, Chen Gang 写道:
>>>
>>> 1. not to send same patch triple times. 
>>
>>   thanks, I shall notice, next time.
>>   (I shall 'believe' another members).
>>
>>> 2. config your email client,because tab is changed to space.
>>>    you can read Documentation/email-clients.txt.
>>
>>   1) thanks. I shall notice, next time.
>>   2) now, I get gvim as extention editor for thounderbird
>>   3) the patch is generated by `git format-patch -s --summary --stat`
>>      it use "' '\t" as head, I do not touch it, maybe it is correct.
>>
>> welcome any members to giving additional suggestions and completions.
>>
>> thanks
>>
>> the modified contents are below,
>> -----------------------------------------------------------------------------------
>>
>>   the length of rt->dst.dev->name is 16 (IFNAMSIZ)
>>   in seq_printf, it is not suitable to use %8s for rt->dst.dev->name.
>>   so change it to %s, since each line has not been solid any more.
>>
>>   additional information:
>>
>>     %8s  limit the width, not for the original string output length
>>          if name length is more than 8, it still can be fully displayed.
>>          if name length is less than 8, the ' ' will be filled before name.
>>
>>     %.8s truly limit the original string output length (precision)
>>
>> Signed-off-by: Chen Gang <gang.chen@...anux.com>
>> ---
>>  net/ipv6/route.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index c42650c..b60bc52 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -2835,7 +2835,7 @@ static int rt6_info_route(struct rt6_info *rt, void *p_arg)
>>  	} else {
>>  		seq_puts(m, "00000000000000000000000000000000");
>>  	}
>> -	seq_printf(m, " %08x %08x %08x %08x %8s\n",
>> +	seq_printf(m, " %08x %08x %08x %08x %s\n",
>>  		   rt->rt6i_metric, atomic_read(&rt->dst.__refcnt),
>>  		   rt->dst.__use, rt->rt6i_flags,
>>  		   rt->dst.dev ? rt->dst.dev->name : "");
>>
>>
>>
> 
> 
> 


-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists