Re: [Patch net-next] pktgen: small code cleanup

[Wei Yang <weiyang@linux.vnet.ibm.com>]

On Thu, Apr 03, 2014 at 01:35:25PM -0400, David Miller wrote:
>From: Wei Yang <weiyang@linux.vnet.ibm.com>
>Date: Wed,  2 Apr 2014 16:24:51 +0800
>
>> Print the warning when the format is not correct.
>> 
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> ---
>>  net/core/pktgen.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
>> index fdac61c..89d33c2 100644
>> --- a/net/core/pktgen.c
>> +++ b/net/core/pktgen.c
>> @@ -1767,7 +1767,7 @@ static ssize_t pktgen_thread_write(struct file *file,
>>  	char *pg_result;
>>  
>>  	if (count < 1) {
>> -		//      sprintf(pg_result, "Wrong command format");
>> +		pr_warn("WARNING: Wrong command format in %s\n", __func__);
>>  		return -EINVAL;
>>  	}
>>  
>
>I don't think a kernel log message is appropriate.
>
>The intention, as per the comment protected code, was to make this
>error message show up in show_results()'s output.

Agree, I guess the author originally intended to put this warning and shows in
show_results(). While since there is a chance that at this moment that the
pktgen_thread doesn't exist, then can't put into the results.

I did a search and try to find who introduce this code. Finally found this
commit 
        e051211 [NET]: pktgen update
in git repo:
        git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git

Unfortunately, this looks like a typo and author forgot to remove it.

At the mean time, I found in proc_if_write(), it print a kernel log message
when the format is not correct. From this log, it shows originally it tried to
save the warning message in the "reslut", but I don't know why it change to
just print a kernel log.

This code is preserved till now in pktgen_if_write(). Actually, I think in
this place we could save it in the pg_result.

So, my suggestion is:
1. in pktgen_thread_write(), remove the code which is commented out
2. in pktgen_if_write(), save warning message in pg_result

Hope my understanding is correct, if not, please let me know :-)

>
>I'm not applying this, sorry.

-- 
Richard Yang
Help you, Help me