linux-kernel - Re: [PATCH] Documentation: checkpatch: Tweak BIT() macro include

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 May 2021 19:14:48 +0930
From:   "Andrew Jeffery" <andrew@...id.au>
To:     "Dwaipayan Ray" <dwaipayanray1@...il.com>
Cc:     "Lukas Bulwahn" <lukas.bulwahn@...il.com>,
        "Linux Doc Mailing List" <linux-doc@...r.kernel.org>,
        "Joe Perches" <joe@...ches.com>,
        "Jonathan Corbet" <corbet@....net>,
        "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
        openbmc@...ts.ozlabs.org, "Jiri Slaby" <jirislaby@...nel.org>
Subject: Re: [PATCH] Documentation: checkpatch: Tweak BIT() macro include



On Thu, 20 May 2021, at 18:47, Dwaipayan Ray wrote:
> On Thu, May 20, 2021 at 12:55 PM Andrew Jeffery <andrew@...id.au> wrote:
> >
> >
> >
> > On Thu, 20 May 2021, at 16:28, Lukas Bulwahn wrote:
> > > On Thu, May 20, 2021 at 3:57 AM Andrew Jeffery <andrew@...id.au> wrote:
> > > >
> > > > While include/linux/bitops.h brings in the BIT() macro, it was moved to
> > > > include/linux/bits.h in [1]. Since [1] BIT() has moved again into
> > > > include/vdso/bits.h via [2].
> > > >
> > > > I think the move to the vDSO header can be considered a implementation
> > > > detail, so for now update the checkpatch documentation to recommend use
> > > > of include/linux/bits.h.
> > > >
> > > > [1] commit 8bd9cb51daac ("locking/atomics, asm-generic: Move some macros from <linux/bitops.h> to a new <linux/bits.h> file")
> > > > [2] commit 3945ff37d2f4 ("linux/bits.h: Extract common header for vDSO")
> > > >
> > > > Cc: Jiri Slaby <jirislaby@...nel.org>
> > > > Signed-off-by: Andrew Jeffery <andrew@...id.au>
> > >
> > > Looks sound to me.
> > >
> > > I would prefer a bit of word-smithing the commit message by just
> > > removing the references:
> > >
> > > So:
> > >
> > > > While include/linux/bitops.h brings in the BIT() macro, it was moved to
> > > > include/linux/bits.h in commit 8bd9cb51daac ("locking/atomics, asm-generic: Move some macros from <linux/bitops.h> to a new <linux/bits.h> file"). Since that commit, BIT() has moved again into
> > > > include/vdso/bits.h via commit 3945ff37d2f4 ("linux/bits.h: Extract common header for vDSO").
> > > >
> > > > I think the move to the vDSO header can be considered a implementation
> > > > detail, so for now update the checkpatch documentation to recommend use
> > > > of include/linux/bits.h.
> > > >
> > >
> > > And then drop references [1] and [2].
> > >
> > > Andrew, what do you think?
> >
> > I mostly did this because initially I wrapped the commit message and
> > checkpatch spat out errors when it failed to properly identify the
> > commit description for [1]. But, leaving the description unwrapped
> > inline in the text feels untidy as it's just a work-around to dodge a
> > shortcoming of checkpatch.
> >
> > With the reference style the long line moves out of the way and
> > checkpatch can identify the commit descriptions, at the expense of
> > complaints about line length instead. But the line length issue was
> > only a warning and so didn't seem quite so critical.
> >
> > While the referencing style is terse I felt it was a reasonable
> > compromise that didn't involve fixing checkpatch to fix the checkpatch
> > documentation :/
> >
> 
> Hey,
> Can you share which wrap around caused the checkpatch errors
> to be emitted? We can try to fix that.
> 
> I was able to wrap it without checkpatch complaining. You might consider
> replacing it with this if you wish?
> 
> While include/linux/bitops.h brings in the BIT() macro, it was moved to
> include/linux/bits.h in commit 8bd9cb51daac ("locking/atomics, asm-generic:
> Move some macros from <linux/bitops.h> to a new <linux/bits.h> file").

This wording works because the commit description is only split across 
two lines. With the wording I had it was split across three, and this 
caused checkpatch to barf. If we do this:

While include/linux/bitops.h brings in the BIT() macro, it was moved to
include/linux/bits.h in commit 8bd9cb51daac ("locking/atomics, asm-generic:
Move some macros from <linux/bitops.h> to a new <linux/bits.h>
file").

we get:

ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 8bd9cb51daac ("locking/atomics, asm-generic: Move some macros from <linux/bitops.h> to a new <linux/bits.h> file")'
#7: 
include/linux/bits.h in commit 8bd9cb51daac ("locking/atomics, asm-generic:

total: 1 errors, 0 warnings, 8 lines checked

Anyway, I've replaced the commit message with your suggestion:

https://lore.kernel.org/linux-doc/20210520093949.511471-1-andrew@aj.id.au/

Thanks for work-shopping it :)

Andrew