RE: [PATCH] checkpatch: add warning on %pk instead of %pK usage

From: Roberts, William C
Date: Wed Feb 15 2017 - 18:49:19 EST




> -----Original Message-----
> From: Joe Perches [mailto:joe@xxxxxxxxxxx]
> Sent: Monday, February 13, 2017 2:21 PM
> To: Roberts, William C <william.c.roberts@xxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; apw@xxxxxxxxxxxxx; kernel-
> hardening@xxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH] checkpatch: add warning on %pk instead of %pK usage
>
> (Adding back the cc's)
>
> On Mon, 2017-02-13 at 21:28 +0000, Roberts, William C wrote:
> > <snip>
> > > No worries.
> > > No idea why it doesn't work for you.
> > > Maybe the hand applying was somehow
> > > faulty?
> > >
> > > The attached is on top of -next so it does have offsets on Linus'
> > > tree, but it seems to work.
> > >
> > > (on -linux)
> > >
> > > $ patch -p1 < cp_vsp.diff
> > > patching file scripts/checkpatch.pl
> > > Hunk #1 succeeded at 5634 (offset -36 lines).
> > >
> > > $ cat t_block.c
> > > {
> > > MY_DEBUG(drv->foo,
> > >  "%pk",
> > >  foo->boo);
> > > }
> > > $ ./scripts/checkpatch.pl -f t_block.c
> > > WARNING: Invalid vsprintf pointer extension '%pk'
> > > #2: FILE: t_block.c:2:
> > > + MY_DEBUG(drv->foo,
> > > +  "%pk",
> > > +  foo->boo);
> > >
> > > total: 0 errors, 1 warnings, 5 lines checked
> > >
> > > NOTE: For some of the reported defects, checkpatch may be able to
> > >       mechanically convert to the typical style using --fix or --fix-inplace.
> > >
> > > t_block.c has style problems, please review.
> > >
> > > NOTE: If any of the errors are false positives, please report
> > >       them to the maintainer, see CHECKPATCH in MAINTAINERS.
> >
> >
> > Applied. It works fine with your example (see attached
> > 0001-tblock.patch) but it doesn't provide Output for me with
> > 0002-drv-hack.patch (attached as well)
> >
> > $ ./scripts/checkpatch.pl 0002-drv-hack.patch
> > total: 0 errors, 0 warnings, 10 lines checked
> >
> > 0002-drv-hack.patch has no obvious style problems and is ready for submission.
> >
> > ./scripts/checkpatch.pl 0001-tblock.patch
> > WARNING: added, moved or deleted file(s), does MAINTAINERS need
> updating?
> > #13:
> > new file mode 100644
> >
> > WARNING: Invalid vsprintf pointer extension '%pk'
> > #19: FILE: t_block.c:2:
> > + MY_DEBUG(drv->foo,
> > + "%pk",
> > + foo->boo);
> >
> > total: 0 errors, 2 warnings, 6 lines checked
> >
> > NOTE: For some of the reported defects, checkpatch may be able to
> > mechanically convert to the typical style using --fix or --fix-inplace.
> >
> > 0001-tblock.patch has style problems, please review.
> >
> > NOTE: If any of the errors are false positives, please report
> > them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> This means _all_ the $stat checks aren't being done on patches that add just a
> single multi-line statement.
>
> Andrew? Any thoughts on how to enable $stat appropriately for patch contexts
> with a single multi-line statement?

I'm for merging your patch as is, and then take up the fact that $stat is not working correctly
as a separate change, does that seem reasonable?