* [PATCH] Documentation: checkpatch: Tweak BIT() macro include @ 2021-05-20 1:57 Andrew Jeffery 2021-05-20 5:24 ` Jiri Slaby 2021-05-20 6:58 ` Lukas Bulwahn 0 siblings, 2 replies; 9+ messages in thread From: Andrew Jeffery @ 2021-05-20 1:57 UTC (permalink / raw) To: linux-doc Cc: dwaipayanray1, lukas.bulwahn, joe, corbet, linux-kernel, openbmc, Jiri Slaby 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@kernel.org> Signed-off-by: Andrew Jeffery <andrew@aj.id.au> --- Documentation/dev-tools/checkpatch.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst index 51fed1bd72ec..59fcc9f627ea 100644 --- a/Documentation/dev-tools/checkpatch.rst +++ b/Documentation/dev-tools/checkpatch.rst @@ -472,7 +472,7 @@ Macros, Attributes and Symbols **BIT_MACRO** Defines like: 1 << <digit> could be BIT(digit). - The BIT() macro is defined in include/linux/bitops.h:: + The BIT() macro is defined via include/linux/bits.h:: #define BIT(nr) (1UL << (nr)) -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Documentation: checkpatch: Tweak BIT() macro include 2021-05-20 1:57 [PATCH] Documentation: checkpatch: Tweak BIT() macro include Andrew Jeffery @ 2021-05-20 5:24 ` Jiri Slaby 2021-05-20 6:58 ` Lukas Bulwahn 1 sibling, 0 replies; 9+ messages in thread From: Jiri Slaby @ 2021-05-20 5:24 UTC (permalink / raw) To: Andrew Jeffery, linux-doc Cc: dwaipayanray1, lukas.bulwahn, joe, corbet, linux-kernel, openbmc On 20. 05. 21, 3:57, Andrew Jeffery 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@kernel.org> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Acked-by: Jiri Slaby <jirislaby@kernel.org> Thanks. > --- > Documentation/dev-tools/checkpatch.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst > index 51fed1bd72ec..59fcc9f627ea 100644 > --- a/Documentation/dev-tools/checkpatch.rst > +++ b/Documentation/dev-tools/checkpatch.rst > @@ -472,7 +472,7 @@ Macros, Attributes and Symbols > > **BIT_MACRO** > Defines like: 1 << <digit> could be BIT(digit). > - The BIT() macro is defined in include/linux/bitops.h:: > + The BIT() macro is defined via include/linux/bits.h:: > > #define BIT(nr) (1UL << (nr)) > > -- js suse labs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Documentation: checkpatch: Tweak BIT() macro include 2021-05-20 1:57 [PATCH] Documentation: checkpatch: Tweak BIT() macro include Andrew Jeffery 2021-05-20 5:24 ` Jiri Slaby @ 2021-05-20 6:58 ` Lukas Bulwahn 2021-05-20 7:24 ` Andrew Jeffery 1 sibling, 1 reply; 9+ messages in thread From: Lukas Bulwahn @ 2021-05-20 6:58 UTC (permalink / raw) To: Andrew Jeffery Cc: open list:DOCUMENTATION, Dwaipayan Ray, Joe Perches, Jonathan Corbet, Linux Kernel Mailing List, openbmc, Jiri Slaby On Thu, May 20, 2021 at 3:57 AM Andrew Jeffery <andrew@aj.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@kernel.org> > Signed-off-by: Andrew Jeffery <andrew@aj.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? Lukas ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Documentation: checkpatch: Tweak BIT() macro include 2021-05-20 6:58 ` Lukas Bulwahn @ 2021-05-20 7:24 ` Andrew Jeffery 2021-05-20 9:17 ` Dwaipayan Ray 0 siblings, 1 reply; 9+ messages in thread From: Andrew Jeffery @ 2021-05-20 7:24 UTC (permalink / raw) To: Lukas Bulwahn Cc: Linux Doc Mailing List, Dwaipayan Ray, Joe Perches, Jonathan Corbet, Linux Kernel Mailing List, openbmc, Jiri Slaby On Thu, 20 May 2021, at 16:28, Lukas Bulwahn wrote: > On Thu, May 20, 2021 at 3:57 AM Andrew Jeffery <andrew@aj.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@kernel.org> > > Signed-off-by: Andrew Jeffery <andrew@aj.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 :/ Andrew ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Documentation: checkpatch: Tweak BIT() macro include 2021-05-20 7:24 ` Andrew Jeffery @ 2021-05-20 9:17 ` Dwaipayan Ray 2021-05-20 9:44 ` Andrew Jeffery 0 siblings, 1 reply; 9+ messages in thread From: Dwaipayan Ray @ 2021-05-20 9:17 UTC (permalink / raw) To: Andrew Jeffery Cc: Lukas Bulwahn, Linux Doc Mailing List, Joe Perches, Jonathan Corbet, Linux Kernel Mailing List, openbmc, Jiri Slaby On Thu, May 20, 2021 at 12:55 PM Andrew Jeffery <andrew@aj.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@aj.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@kernel.org> > > > Signed-off-by: Andrew Jeffery <andrew@aj.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"). 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 an implementation detail, so for now update the checkpatch documentation to recommend use of include/linux/bits.h. Thanks, Dwaipayan. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Documentation: checkpatch: Tweak BIT() macro include 2021-05-20 9:17 ` Dwaipayan Ray @ 2021-05-20 9:44 ` Andrew Jeffery 2021-05-20 10:21 ` Dwaipayan Ray 0 siblings, 1 reply; 9+ messages in thread From: Andrew Jeffery @ 2021-05-20 9:44 UTC (permalink / raw) To: Dwaipayan Ray Cc: Lukas Bulwahn, Linux Doc Mailing List, Joe Perches, Jonathan Corbet, Linux Kernel Mailing List, openbmc, Jiri Slaby On Thu, 20 May 2021, at 18:47, Dwaipayan Ray wrote: > On Thu, May 20, 2021 at 12:55 PM Andrew Jeffery <andrew@aj.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@aj.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@kernel.org> > > > > Signed-off-by: Andrew Jeffery <andrew@aj.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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Documentation: checkpatch: Tweak BIT() macro include 2021-05-20 9:44 ` Andrew Jeffery @ 2021-05-20 10:21 ` Dwaipayan Ray 2021-05-20 12:06 ` Lukas Bulwahn 0 siblings, 1 reply; 9+ messages in thread From: Dwaipayan Ray @ 2021-05-20 10:21 UTC (permalink / raw) To: Andrew Jeffery Cc: Lukas Bulwahn, Linux Doc Mailing List, Joe Perches, Jonathan Corbet, Linux Kernel Mailing List, openbmc, Jiri Slaby On Thu, May 20, 2021 at 3:15 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > On Thu, 20 May 2021, at 18:47, Dwaipayan Ray wrote: > > On Thu, May 20, 2021 at 12:55 PM Andrew Jeffery <andrew@aj.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@aj.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@kernel.org> > > > > > Signed-off-by: Andrew Jeffery <andrew@aj.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: > Yes it won't work for 3 lines. We are checking only for an additional line for split commit descriptions. Might be a thing to improve in the future. > 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 :) > Thanks for the patch :) Dwaipayan. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Documentation: checkpatch: Tweak BIT() macro include 2021-05-20 10:21 ` Dwaipayan Ray @ 2021-05-20 12:06 ` Lukas Bulwahn 2021-05-20 13:02 ` Dwaipayan Ray 0 siblings, 1 reply; 9+ messages in thread From: Lukas Bulwahn @ 2021-05-20 12:06 UTC (permalink / raw) To: Dwaipayan Ray Cc: Andrew Jeffery, Linux Doc Mailing List, Joe Perches, Jonathan Corbet, Linux Kernel Mailing List, openbmc, Jiri Slaby On Thu, May 20, 2021 at 12:21 PM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote: > > On Thu, May 20, 2021 at 3:15 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > > > > > On Thu, 20 May 2021, at 18:47, Dwaipayan Ray wrote: > > > On Thu, May 20, 2021 at 12:55 PM Andrew Jeffery <andrew@aj.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@aj.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@kernel.org> > > > > > > Signed-off-by: Andrew Jeffery <andrew@aj.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: > > > > Yes it won't work for 3 lines. We are checking only for an additional line > for split commit descriptions. Might be a thing to improve in the future. > Dwaipayan, you certainly got my go to improve checkpatch for this issue. You might want to re-run our known checkpatch evaluation and see how often this issue for commit references with multiple lines appears. Looking forward to your patch, Lukas ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Documentation: checkpatch: Tweak BIT() macro include 2021-05-20 12:06 ` Lukas Bulwahn @ 2021-05-20 13:02 ` Dwaipayan Ray 0 siblings, 0 replies; 9+ messages in thread From: Dwaipayan Ray @ 2021-05-20 13:02 UTC (permalink / raw) To: Lukas Bulwahn; +Cc: Andrew Jeffery, Joe Perches, Linux Kernel Mailing List On Thu, May 20, 2021 at 5:36 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > On Thu, May 20, 2021 at 12:21 PM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote: > > > > On Thu, May 20, 2021 at 3:15 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > > > > > > > > > On Thu, 20 May 2021, at 18:47, Dwaipayan Ray wrote: > > > > On Thu, May 20, 2021 at 12:55 PM Andrew Jeffery <andrew@aj.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@aj.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@kernel.org> > > > > > > > Signed-off-by: Andrew Jeffery <andrew@aj.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: > > > > > > > Yes it won't work for 3 lines. We are checking only for an additional line > > for split commit descriptions. Might be a thing to improve in the future. > > > > Dwaipayan, you certainly got my go to improve checkpatch for this > issue. You might want to re-run our known checkpatch evaluation and > see how often this issue for commit references with multiple lines > appears. > > Looking forward to your patch, Sure I will try something. Last time I ran checkpatch over 50k commits from v5.4 there were 1032 instances of the error "GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1>". Ref: https://raydwaipayan.github.io/blogs/checkpatch_out_50k.txt (42MB dump) But now it's hard to tell how many are due to warping into > 2 lines. A majority of these seem to be actual errors. (Through random sampling :)). But unless we extract the commit messages themselves it's hard to tell. Might be a very insignificant number or might be considerable as well. Does Joe have any suggestions for this? Thanks, Dwaipayan. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-05-20 13:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-20 1:57 [PATCH] Documentation: checkpatch: Tweak BIT() macro include Andrew Jeffery 2021-05-20 5:24 ` Jiri Slaby 2021-05-20 6:58 ` Lukas Bulwahn 2021-05-20 7:24 ` Andrew Jeffery 2021-05-20 9:17 ` Dwaipayan Ray 2021-05-20 9:44 ` Andrew Jeffery 2021-05-20 10:21 ` Dwaipayan Ray 2021-05-20 12:06 ` Lukas Bulwahn 2021-05-20 13:02 ` Dwaipayan Ray
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).