Re: [PATCH net-next] net/trace: fix printk format in inet_sock_set_state

On Sat, Dec 23, 2017 at 5:42 PM, Sergei Shtylyov
[off-list ref] wrote:

On 12/23/2017 4:10 AM, Yafang Shao wrote:

quoted

quoted

quoted

There's a space character missed in the printk messages.
This error should be prevented with checkscript.pl, but it couldn't
caught

quoted

It is checkpatch.pl.

   Yes, that too. But I actually meant you missed "be" between "couldn't"
and "caught"...

quoted

quoted

quoted

by running with "checkscript.pl -f xxxx.patch", that's what I had run
before.
What a carelessness.

quoted

quoted

    You generally don't need to break up the messages violating 80-column
limit, and checkpatch.pl should be aware of this...

quoted

Oh. That's right.
It can be aware of that.

   It is aware --- I'm just not sure it recognizes TP_printk() -- like it
does recognize printk() fo that purpose.

It recognizes TP_printk as well. I have verified.

quoted

I just want to make the code easy to read and limit the textwidth to
80 character.

   Contrariwise, that's what you shouldn't do. Would simplify searching for
the messages in the kernel source.

Agree to that.

quoted

If the message takes two lines as bellow,
     printk("xxx "
                      ^ space character.
               "yyy");
The checkpatch.pl  could also be aware of that if the first line is
not end with space character, but it couldn't be aware of that if run
with "checkpatch.pl -f xxxx.patch".

   Option -f tells checkpatch.pl that it should check a source file, not a
patch. I don't know why you use that with the patches...

Because the default option "--patch" sometimes shows some unnecessary warnings.
next time I will not use '-f' option.

quoted

quoted

quoted

Fixes: 563e0bb0dc74("net: tracepoint: replace tcp_set_state tracepoint
with
inet_sock_set_state tracepoint")
Signed-off-by: Yafang Shao <redacted>

[...]

MBR, Sergei