* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
[not found] ` <alpine.DEB.2.22.394.2008201856110.2524@hadrien>
@ 2020-08-22 1:08 ` Joe Perches
2020-08-22 3:35 ` Valdis Klētnieks
0 siblings, 1 reply; 15+ messages in thread
From: Joe Perches @ 2020-08-22 1:08 UTC (permalink / raw)
To: Julia Lawall, kernel-janitors, kernelnewbies, linux-kernel-mentees
Cc: Giuseppe Scrivano, Andrew Morton, Andy Whitcroft, cocci, LKML
(forwarding on to kernel-janitors/mentees and kernelnewbies)
Just fyi for anyone that cares:
A janitorial task for someone might be to use Julia's coccinelle
script below to convert the existing instances of commas that
separate statements into semicolons.
https://lore.kernel.org/lkml/alpine.DEB.2.22.394.2008201856110.2524@hadrien/
On Thu, 2020-08-20 at 19:03 +0200, Julia Lawall wrote:
> > > I have a bunch of variations of this that are more complicated than I
> > > would have expected. One shorter variant that I have is:
> > >
> > > @@
> > > expression e1,e2;
> > > statement S;
> > > @@
> > >
> > > S
> > > e1
> > > -,
> > > +;
> > > (<+... e2 ...+>);
> > >
> > > This will miss cases where the first statement is the comma thing. But I
> > > think it is possible to improve this now. I will check.
> >
> > Hi Julia.
> >
> > Right, thanks, this adds a dependency on a statement
> > before the expression. Any stragglers would be easily
> > found using slightly different form.
> > There are not very many of these in linux kernel.
> >
> > Another nicety would be to allow the s/,/;/ conversion to
> > find both b and c in this sequence:
> > a = 1;
> > b = 2,
> > c = 3,
> > d = 4;
> > without running the script multiple times.
> > There are many dozen uses of this style in linux kernel.
> >
> > I tried variants of adding a comma after the e2 expression,
> > but cocci seems to have parsing problems with:
> >
> > @@
> > expression e1;
> > expression e2;
> > @@
> > e1
> > - ,
> > + ;
> > e2,
>
> This doesn't work because it's not a valid expression.
>
> The problem is solved by doing:
>
> e1
> - ,
> + ;
> e2
> ... when any
>
> But that doesn't work in the current version of Coccinelle. I have fixed
> the problem, though and it will work shortly.
>
> > I do appreciate that coccinelle adds braces for multiple
> > expression comma use after an if.
> >
> > i.e.:
> > if (foo)
> > a = 1, b = 2;
> > becomes
> > if (foo) {
> > a = 1; b = 2;
> > }
>
> I wasn't sure what was wanted for such things. Should b = 2 now be on a
> separate line?
>
> I took the strategy of avoiding the problem and leaving those cases as is.
> That also solves the LIST_HEAD problem. But if it is wanted to change
> these commas under ifs, then that is probably possible too.
>
> My current complete solution is as follows. The first rule avoids changing
> commas in macros, where thebody of the macro is just an expression. The
> second rule uses position variables to ensure that the two expression are
> on different lines.
>
> @r@
> expression e1,e2;
> statement S;
> position p;
> @@
>
> e1 ,@S@p e2;
>
> @@
> expression e1,e2;
> position p1;
> position p2 :
> script:ocaml(p1) { not((List.hd p1).line_end = (List.hd p2).line) };
> statement S;
> position r.p;
> @@
>
> e1@p1
> -,@S@p
> +;
> e2@p2
> ... when any
>
> The generated patch is below.
>
> julia
>
> diff -u -p a/drivers/reset/hisilicon/reset-hi3660.c b/drivers/reset/hisilicon/reset-hi3660.c
> --- a/drivers/reset/hisilicon/reset-hi3660.c
> +++ b/drivers/reset/hisilicon/reset-hi3660.c
> @@ -89,7 +89,7 @@ static int hi3660_reset_probe(struct pla
> return PTR_ERR(rc->map);
> }
>
> - rc->rst.ops = &hi3660_reset_ops,
> + rc->rst.ops = &hi3660_reset_ops;
> rc->rst.of_node = np;
> rc->rst.of_reset_n_cells = 2;
> rc->rst.of_xlate = hi3660_reset_xlate;
The rest of the changes are in the link above...
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-08-22 1:08 ` [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma Joe Perches
@ 2020-08-22 3:35 ` Valdis Klētnieks
2020-08-22 5:30 ` Joe Perches
0 siblings, 1 reply; 15+ messages in thread
From: Valdis Klētnieks @ 2020-08-22 3:35 UTC (permalink / raw)
To: Joe Perches
Cc: Giuseppe Scrivano, kernelnewbies, kernel-janitors, LKML,
Andy Whitcroft, Andrew Morton, linux-kernel-mentees, cocci
[-- Attachment #1: Type: text/plain, Size: 838 bytes --]
On Fri, 21 Aug 2020 18:08:08 -0700, Joe Perches said:
> (forwarding on to kernel-janitors/mentees and kernelnewbies)
>
> Just fyi for anyone that cares:
>
> A janitorial task for someone might be to use Julia's coccinelle
> script below to convert the existing instances of commas that
> separate statements into semicolons.
Note that you need to *really* check for possible changes in semantics.
It's *usually* OK to do that, but sometimes it's not...
for (i=0; i++, last++; !last) {
changing that comma to a ; will break the compile. In other cases, it can
introduce subtle bugs.
> > I do appreciate that coccinelle adds braces for multiple
> > expression comma use after an if.
> >
> > i.e.:
> > if (foo)
> > a = 1, b = 2;
> > becomes
> > if (foo) {
> > a = 1; b = 2;
> > }
Yeah. Like there, if you forget to add the {}.
[-- Attachment #2: Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-08-22 3:35 ` Valdis Klētnieks
@ 2020-08-22 5:30 ` Joe Perches
2020-08-22 7:07 ` Julia Lawall
0 siblings, 1 reply; 15+ messages in thread
From: Joe Perches @ 2020-08-22 5:30 UTC (permalink / raw)
To: Valdis Klētnieks
Cc: Giuseppe Scrivano, kernelnewbies, kernel-janitors, LKML,
Andy Whitcroft, Andrew Morton, linux-kernel-mentees, cocci
On Fri, 2020-08-21 at 23:35 -0400, Valdis Klētnieks wrote:
> On Fri, 21 Aug 2020 18:08:08 -0700, Joe Perches said:
> > (forwarding on to kernel-janitors/mentees and kernelnewbies)
> >
> > Just fyi for anyone that cares:
> >
> > A janitorial task for someone might be to use Julia's coccinelle
> > script below to convert the existing instances of commas that
> > separate statements into semicolons.
>
> Note that you need to *really* check for possible changes in semantics.
> It's *usually* OK to do that, but sometimes it's not...
>
> for (i=0; i++, last++; !last) {
>
> changing that comma to a ; will break the compile. In other cases, it can
> introduce subtle bugs.
True enough for a general statement, though the coccinelle
script Julia provided does not change a single instance of
for loop expressions with commas.
As far as I can tell, no logic defect is introduced by the
script at all.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-08-22 5:30 ` Joe Perches
@ 2020-08-22 7:07 ` Julia Lawall
2020-09-24 20:19 ` Thomas Gleixner
0 siblings, 1 reply; 15+ messages in thread
From: Julia Lawall @ 2020-08-22 7:07 UTC (permalink / raw)
To: Joe Perches
Cc: Giuseppe Scrivano, Valdis Klētnieks, kernelnewbies,
kernel-janitors, LKML, Andy Whitcroft, Andrew Morton,
linux-kernel-mentees, cocci
[-- Attachment #1: Type: text/plain, Size: 1245 bytes --]
On Fri, 21 Aug 2020, Joe Perches wrote:
> On Fri, 2020-08-21 at 23:35 -0400, Valdis Klētnieks wrote:
> > On Fri, 21 Aug 2020 18:08:08 -0700, Joe Perches said:
> > > (forwarding on to kernel-janitors/mentees and kernelnewbies)
> > >
> > > Just fyi for anyone that cares:
> > >
> > > A janitorial task for someone might be to use Julia's coccinelle
> > > script below to convert the existing instances of commas that
> > > separate statements into semicolons.
> >
> > Note that you need to *really* check for possible changes in semantics.
> > It's *usually* OK to do that, but sometimes it's not...
> >
> > for (i=0; i++, last++; !last) {
> >
> > changing that comma to a ; will break the compile. In other cases, it can
> > introduce subtle bugs.
>
> True enough for a general statement, though the coccinelle
> script Julia provided does not change a single instance of
> for loop expressions with commas.
>
> As far as I can tell, no logic defect is introduced by the
> script at all.
The script has a rule to ensure that what is changed is part of a top
level statement that has the form e1, e2;. I put that in to avoid
transforming cases where the comma is the body of a macro, but it protects
against for loop headers as well.
julia
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-08-22 7:07 ` Julia Lawall
@ 2020-09-24 20:19 ` Thomas Gleixner
2020-09-24 20:21 ` Julia Lawall
2020-09-24 20:33 ` Joe Perches
0 siblings, 2 replies; 15+ messages in thread
From: Thomas Gleixner @ 2020-09-24 20:19 UTC (permalink / raw)
To: Julia Lawall, Joe Perches
Cc: Giuseppe Scrivano, Valdis Klētnieks, kernelnewbies,
kernel-janitors, LKML, Andy Whitcroft, Andrew Morton,
linux-kernel-mentees, cocci
On Sat, Aug 22 2020 at 09:07, Julia Lawall wrote:
> On Fri, 21 Aug 2020, Joe Perches wrote:
>> True enough for a general statement, though the coccinelle
>> script Julia provided does not change a single instance of
>> for loop expressions with commas.
>>
>> As far as I can tell, no logic defect is introduced by the
>> script at all.
>
> The script has a rule to ensure that what is changed is part of a top
> level statement that has the form e1, e2;. I put that in to avoid
> transforming cases where the comma is the body of a macro, but it protects
> against for loop headers as well.
Right. I went through the lot and did not find something dodgy. Except
for two hunks this still applies. Can someone please send a proper patch
with changelog/SOB etc. for this?
And of course that script really wants to be part of the kernel cocci
checks to catch further instances.
Thanks,
tglx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-09-24 20:19 ` Thomas Gleixner
@ 2020-09-24 20:21 ` Julia Lawall
2020-09-24 20:33 ` Joe Perches
1 sibling, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2020-09-24 20:21 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Giuseppe Scrivano, Valdis Klētnieks, kernelnewbies,
kernel-janitors, LKML, Andy Whitcroft, Joe Perches,
Andrew Morton, linux-kernel-mentees, cocci
On Thu, 24 Sep 2020, Thomas Gleixner wrote:
> On Sat, Aug 22 2020 at 09:07, Julia Lawall wrote:
> > On Fri, 21 Aug 2020, Joe Perches wrote:
> >> True enough for a general statement, though the coccinelle
> >> script Julia provided does not change a single instance of
> >> for loop expressions with commas.
> >>
> >> As far as I can tell, no logic defect is introduced by the
> >> script at all.
> >
> > The script has a rule to ensure that what is changed is part of a top
> > level statement that has the form e1, e2;. I put that in to avoid
> > transforming cases where the comma is the body of a macro, but it protects
> > against for loop headers as well.
>
> Right. I went through the lot and did not find something dodgy. Except
> for two hunks this still applies. Can someone please send a proper patch
> with changelog/SOB etc. for this?
>
> And of course that script really wants to be part of the kernel cocci
> checks to catch further instances.
I will try to get to it soon. Thanks for checking all the cases.
julia
>
> Thanks,
>
> tglx
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-09-24 20:19 ` Thomas Gleixner
2020-09-24 20:21 ` Julia Lawall
@ 2020-09-24 20:33 ` Joe Perches
2020-09-24 21:53 ` Thomas Gleixner
1 sibling, 1 reply; 15+ messages in thread
From: Joe Perches @ 2020-09-24 20:33 UTC (permalink / raw)
To: Thomas Gleixner, Julia Lawall
Cc: Giuseppe Scrivano, Valdis Klētnieks, kernelnewbies,
kernel-janitors, LKML, Andy Whitcroft, Andrew Morton,
linux-kernel-mentees, cocci
On Thu, 2020-09-24 at 22:19 +0200, Thomas Gleixner wrote:
> On Sat, Aug 22 2020 at 09:07, Julia Lawall wrote:
> > On Fri, 21 Aug 2020, Joe Perches wrote:
> > > True enough for a general statement, though the coccinelle
> > > script Julia provided does not change a single instance of
> > > for loop expressions with commas.
> > >
> > > As far as I can tell, no logic defect is introduced by the
> > > script at all.
> >
> > The script has a rule to ensure that what is changed is part of a top
> > level statement that has the form e1, e2;. I put that in to avoid
> > transforming cases where the comma is the body of a macro, but it protects
> > against for loop headers as well.
>
> Right. I went through the lot and did not find something dodgy. Except
> for two hunks this still applies. Can someone please send a proper patch
> with changelog/SOB etc. for this?
Treewide?
Somebody no doubt would complain, but there
_really should_ be some mechanism for these
trivial and correct treewide changes...
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-09-24 20:33 ` Joe Perches
@ 2020-09-24 21:53 ` Thomas Gleixner
2020-09-24 22:23 ` Joe Perches
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Gleixner @ 2020-09-24 21:53 UTC (permalink / raw)
To: Joe Perches, Julia Lawall
Cc: Giuseppe Scrivano, Valdis Klētnieks, kernelnewbies,
kernel-janitors, LKML, Andy Whitcroft, Andrew Morton,
linux-kernel-mentees, cocci
On Thu, Sep 24 2020 at 13:33, Joe Perches wrote:
> On Thu, 2020-09-24 at 22:19 +0200, Thomas Gleixner wrote:
>> On Sat, Aug 22 2020 at 09:07, Julia Lawall wrote:
>> > On Fri, 21 Aug 2020, Joe Perches wrote:
>> > > True enough for a general statement, though the coccinelle
>> > > script Julia provided does not change a single instance of
>> > > for loop expressions with commas.
>> > >
>> > > As far as I can tell, no logic defect is introduced by the
>> > > script at all.
>> >
>> > The script has a rule to ensure that what is changed is part of a top
>> > level statement that has the form e1, e2;. I put that in to avoid
>> > transforming cases where the comma is the body of a macro, but it protects
>> > against for loop headers as well.
>>
>> Right. I went through the lot and did not find something dodgy. Except
>> for two hunks this still applies. Can someone please send a proper patch
>> with changelog/SOB etc. for this?
>
> Treewide?
>
> Somebody no doubt would complain, but there
> _really should_ be some mechanism for these
> trivial and correct treewide changes...
There are lots of mechanisms:
- Andrew picks such changes up
- With a few competent eyeballs on it (reviewers) this can go thorugh
the trivial tree as well. It's more than obvious after all.
- Send the script to Linus with a proper change log attached and ask
him to run it.
- In the worst case if nobody feels responsible, I'll take care.
All of the above is better than trying to get the attention of a
gazillion of maintainters.
Thanks,
tglx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-09-24 21:53 ` Thomas Gleixner
@ 2020-09-24 22:23 ` Joe Perches
2020-09-25 17:06 ` Julia Lawall
2020-09-27 17:08 ` Julia Lawall
0 siblings, 2 replies; 15+ messages in thread
From: Joe Perches @ 2020-09-24 22:23 UTC (permalink / raw)
To: Thomas Gleixner, Julia Lawall
Cc: Giuseppe Scrivano, Valdis Klētnieks, kernelnewbies,
kernel-janitors, LKML, Andy Whitcroft, Andrew Morton,
linux-kernel-mentees, cocci
On Thu, 2020-09-24 at 23:53 +0200, Thomas Gleixner wrote:
> On Thu, Sep 24 2020 at 13:33, Joe Perches wrote:
> > On Thu, 2020-09-24 at 22:19 +0200, Thomas Gleixner wrote:
> > > On Sat, Aug 22 2020 at 09:07, Julia Lawall wrote:
> > > > On Fri, 21 Aug 2020, Joe Perches wrote:
> > > > > True enough for a general statement, though the coccinelle
> > > > > script Julia provided does not change a single instance of
> > > > > for loop expressions with commas.
> > > > >
> > > > > As far as I can tell, no logic defect is introduced by the
> > > > > script at all.
> > > >
> > > > The script has a rule to ensure that what is changed is part of a top
> > > > level statement that has the form e1, e2;. I put that in to avoid
> > > > transforming cases where the comma is the body of a macro, but it protects
> > > > against for loop headers as well.
> > >
> > > Right. I went through the lot and did not find something dodgy. Except
> > > for two hunks this still applies. Can someone please send a proper patch
> > > with changelog/SOB etc. for this?
> >
> > Treewide?
> >
> > Somebody no doubt would complain, but there
> > _really should_ be some mechanism for these
> > trivial and correct treewide changes...
>
> There are lots of mechanisms:
I've tried them all.
None of them work particularly well,
especially the individual patch route.
> - Andrew picks such changes up
Generally not treewide.
> - With a few competent eyeballs on it (reviewers) this can go thorugh
> the trivial tree as well. It's more than obvious after all.
Jiri is almost non-existent when it comes to
trivial treewide patches.
> - Send the script to Linus with a proper change log attached and ask
> him to run it.
Linus has concerns about backports and what he
deems trivialities. Generally overblown IMO.
> - In the worst case if nobody feels responsible, I'll take care.
If Julia doesn't send a new patch in the next few
days, I will do the apply, fixup and resend of hers.
So, you're on-deck, nearly up...
> All of the above is better than trying to get the attention of a
> gazillion of maintainters.
True.
And all of the treewide changes depend on some
generic acceptance of value in the type of change.
Some believe that comma->semicolon conversions
aren't useful as there isn't a logical change and
the compiler output wouldn't be different.
Anyway, cheers, Joe
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-09-24 22:23 ` Joe Perches
@ 2020-09-25 17:06 ` Julia Lawall
2020-09-25 17:26 ` Joe Perches
2020-09-27 17:08 ` Julia Lawall
1 sibling, 1 reply; 15+ messages in thread
From: Julia Lawall @ 2020-09-25 17:06 UTC (permalink / raw)
To: Joe Perches
Cc: Giuseppe Scrivano, Valdis Klētnieks, kernelnewbies,
kernel-janitors, LKML, cocci, Andy Whitcroft, Thomas Gleixner,
linux-kernel-mentees, Andrew Morton
On Thu, 24 Sep 2020, Joe Perches wrote:
> On Thu, 2020-09-24 at 23:53 +0200, Thomas Gleixner wrote:
> > On Thu, Sep 24 2020 at 13:33, Joe Perches wrote:
> > > On Thu, 2020-09-24 at 22:19 +0200, Thomas Gleixner wrote:
> > > > On Sat, Aug 22 2020 at 09:07, Julia Lawall wrote:
> > > > > On Fri, 21 Aug 2020, Joe Perches wrote:
> > > > > > True enough for a general statement, though the coccinelle
> > > > > > script Julia provided does not change a single instance of
> > > > > > for loop expressions with commas.
> > > > > >
> > > > > > As far as I can tell, no logic defect is introduced by the
> > > > > > script at all.
> > > > >
> > > > > The script has a rule to ensure that what is changed is part of a top
> > > > > level statement that has the form e1, e2;. I put that in to avoid
> > > > > transforming cases where the comma is the body of a macro, but it protects
> > > > > against for loop headers as well.
> > > >
> > > > Right. I went through the lot and did not find something dodgy. Except
> > > > for two hunks this still applies. Can someone please send a proper patch
> > > > with changelog/SOB etc. for this?
> > >
> > > Treewide?
> > >
> > > Somebody no doubt would complain, but there
> > > _really should_ be some mechanism for these
> > > trivial and correct treewide changes...
> >
> > There are lots of mechanisms:
>
> I've tried them all.
>
> None of them work particularly well,
> especially the individual patch route.
>
> > - Andrew picks such changes up
>
> Generally not treewide.
>
> > - With a few competent eyeballs on it (reviewers) this can go thorugh
> > the trivial tree as well. It's more than obvious after all.
>
> Jiri is almost non-existent when it comes to
> trivial treewide patches.
>
> > - Send the script to Linus with a proper change log attached and ask
> > him to run it.
>
> Linus has concerns about backports and what he
> deems trivialities. Generally overblown IMO.
>
> > - In the worst case if nobody feels responsible, I'll take care.
>
> If Julia doesn't send a new patch in the next few
> days, I will do the apply, fixup and resend of hers.
>
> So, you're on-deck, nearly up...
>
> > All of the above is better than trying to get the attention of a
> > gazillion of maintainters.
>
> True.
>
> And all of the treewide changes depend on some
> generic acceptance of value in the type of change.
>
> Some believe that comma->semicolon conversions
> aren't useful as there isn't a logical change and
> the compiler output wouldn't be different.
I have a script that will cut up the patches and send them to the
appropriate maintainers, so I have no problem with that route.
julia
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-09-25 17:06 ` Julia Lawall
@ 2020-09-25 17:26 ` Joe Perches
2020-09-26 19:11 ` Valdis Klētnieks
0 siblings, 1 reply; 15+ messages in thread
From: Joe Perches @ 2020-09-25 17:26 UTC (permalink / raw)
To: Julia Lawall
Cc: Giuseppe Scrivano, Valdis Klētnieks, kernelnewbies,
kernel-janitors, LKML, cocci, Andy Whitcroft, Thomas Gleixner,
linux-kernel-mentees, Andrew Morton
On Fri, 2020-09-25 at 19:06 +0200, Julia Lawall wrote:
> On Thu, 24 Sep 2020, Joe Perches wrote:
> > On Thu, 2020-09-24 at 23:53 +0200, Thomas Gleixner wrote:
> > > On Thu, Sep 24 2020 at 13:33, Joe Perches wrote:
> > > > On Thu, 2020-09-24 at 22:19 +0200, Thomas Gleixner wrote:
> > > > > On Sat, Aug 22 2020 at 09:07, Julia Lawall wrote:
> > > > > > On Fri, 21 Aug 2020, Joe Perches wrote:
> > > > > > > True enough for a general statement, though the coccinelle
> > > > > > > script Julia provided does not change a single instance of
> > > > > > > for loop expressions with commas.
> > > > > > >
> > > > > > > As far as I can tell, no logic defect is introduced by the
> > > > > > > script at all.
> > > > > >
> > > > > > The script has a rule to ensure that what is changed is part of a top
> > > > > > level statement that has the form e1, e2;. I put that in to avoid
> > > > > > transforming cases where the comma is the body of a macro, but it protects
> > > > > > against for loop headers as well.
> > > > >
> > > > > Right. I went through the lot and did not find something dodgy. Except
> > > > > for two hunks this still applies. Can someone please send a proper patch
> > > > > with changelog/SOB etc. for this?
> > > >
> > > > Treewide?
> > > >
> > > > Somebody no doubt would complain, but there
> > > > _really should_ be some mechanism for these
> > > > trivial and correct treewide changes...
> > >
> > > There are lots of mechanisms:
> >
> > I've tried them all.
> >
> > None of them work particularly well,
> > especially the individual patch route.
> >
> > > - Andrew picks such changes up
> >
> > Generally not treewide.
> >
> > > - With a few competent eyeballs on it (reviewers) this can go thorugh
> > > the trivial tree as well. It's more than obvious after all.
> >
> > Jiri is almost non-existent when it comes to
> > trivial treewide patches.
> >
> > > - Send the script to Linus with a proper change log attached and ask
> > > him to run it.
> >
> > Linus has concerns about backports and what he
> > deems trivialities. Generally overblown IMO.
> >
> > > - In the worst case if nobody feels responsible, I'll take care.
> >
> > If Julia doesn't send a new patch in the next few
> > days, I will do the apply, fixup and resend of hers.
> >
> > So, you're on-deck, nearly up...
> >
> > > All of the above is better than trying to get the attention of a
> > > gazillion of maintainters.
> >
> > True.
> >
> > And all of the treewide changes depend on some
> > generic acceptance of value in the type of change.
> >
> > Some believe that comma->semicolon conversions
> > aren't useful as there isn't a logical change and
> > the compiler output wouldn't be different.
>
> I have a script that will cut up the patches and send them to the
> appropriate maintainers, so I have no problem with that route.
I have a script that does that too.
The complaint I get about its use is
"OMG: My specific commit header style isn't followed"
And the generic individual maintainer apply rate for
each specific patch is always less than 50%.
For instance the patches that converted the comma uses
in if/do/while statements to use braces and semicolons
from a month ago:
https://lore.kernel.org/lkml/cover.1598331148.git.joe@perches.com/
29 patches, 13 applied.
Best of luck.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-09-25 17:26 ` Joe Perches
@ 2020-09-26 19:11 ` Valdis Klētnieks
0 siblings, 0 replies; 15+ messages in thread
From: Valdis Klētnieks @ 2020-09-26 19:11 UTC (permalink / raw)
To: Joe Perches
Cc: Giuseppe Scrivano, kernelnewbies, kernel-janitors, LKML, cocci,
Andy Whitcroft, Thomas Gleixner, linux-kernel-mentees,
Andrew Morton
[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]
On Fri, 25 Sep 2020 10:26:27 -0700, Joe Perches said:
> And the generic individual maintainer apply rate for
> each specific patch is always less than 50%.
>
> For instance the patches that converted the comma uses
> in if/do/while statements to use braces and semicolons
> from a month ago:
> 29 patches, 13 applied.
To be fair, it's *always* been hard to get pure style patches applied, because
they usually hit one of two types of code, with different results:
Some of them hit code that's been stable for a long time - and those patches
don't get applied because of the (admittedly small) risk that a "style" patch
may actually break something - yes, that *does* happen often enough to worry a
risk-adverse subtree maintainer.
Some of them hit code that's actively being worked on - and those patches don't
get applied because they can cause merge conflicts.
This is a hard problem to fix, because it's difficult to say that either of
those viewpoints is *totally* wrong. At best, you can make the case that some
maintainers are a tad over-zealous on their attitude. And since its *hard* to
find good maintainers, it's not possible to fix the problem by just putting
somebody else in charge of a subtree. It's theoretically possible to bypass a
problematic maintainer by sending the patch to the person one level up, or
directly to Linus - but although that usually works if you have an urgent patch
and the maintainer is on vacation or stubborn or whatever, that's got
essentially zero chance of succeeding for a mere style patch.
Unfortunately, although I understand the problem, I don't have a solution. It's
easy to tactfully say "this code is wrong, and here is the fix". It's a lot
harder to find a tactful way to say "This person is wrong and should do it this
way", because code doesn't fight back when you offer constructive criticism....
[-- Attachment #2: Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-09-24 22:23 ` Joe Perches
2020-09-25 17:06 ` Julia Lawall
@ 2020-09-27 17:08 ` Julia Lawall
2020-09-27 17:45 ` Joe Perches
1 sibling, 1 reply; 15+ messages in thread
From: Julia Lawall @ 2020-09-27 17:08 UTC (permalink / raw)
To: Joe Perches
Cc: Giuseppe Scrivano, Valdis Klētnieks, kernelnewbies,
kernel-janitors, LKML, cocci, Andy Whitcroft, Thomas Gleixner,
linux-kernel-mentees, Andrew Morton
I end up with 208 patches. I'm not sure that sending them all at once
would be a good idea...
julia
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-09-27 17:08 ` Julia Lawall
@ 2020-09-27 17:45 ` Joe Perches
2020-09-27 19:35 ` Julia Lawall
0 siblings, 1 reply; 15+ messages in thread
From: Joe Perches @ 2020-09-27 17:45 UTC (permalink / raw)
To: Julia Lawall
Cc: Giuseppe Scrivano, Gustavo A. R. Silva, Valdis Klētnieks,
lkp, kernelnewbies, kernel-janitors, LKML, cocci, Andy Whitcroft,
Thomas Gleixner, linux-kernel-mentees, Andrew Morton
On Sun, 2020-09-27 at 19:08 +0200, Julia Lawall wrote:
> I end up with 208 patches. I'm not sure that sending them all at once
> would be a good idea...
Last I looked the diffstat for comma -> semicolon was:
234 files changed, 509 insertions(+), 509 deletions(-)
So it would be nearly 1 patch per individual file,
Greg KH does send hundreds of patches for -stable at a time.
So, maybe or maybe not send them all at once.
Maybe send it in batches of 25 or so.
There's no single right way to do this.
Maybe put up a git tree somewhere and let the
kernel-robot test compilation.
(A nicety might be for the kernel-robot to have some
option to test pre and post compilation object code
differences with an optional report)
When I automated 491 patches for /* fallthrough */ to
fallthrough;, the robot caught a couple problems which
was great.
https://repo.or.cz/linux-2.6/trivial-mods.git/shortlog/refs/heads/20200310_fallthrough_2
I only posted the first ~30 patches though with
about 50% acceptance. Gustavo Silva picked up the
effort and did a great job. Eventually, a single
treewide patch was posted and accepted by Linus for
this though after dozens of individual patches went
through various maintainer trees:
$ git log --shortstat -1 df561f6688fe
commit df561f6688fef775baa341a0f5d960becd248b11
Author: Gustavo A. R. Silva <gustavoars@kernel.org>
Date: Sun Aug 23 17:36:59 2020 -0500
treewide: Use fallthrough pseudo-keyword
Replace the existing /* fall through */ comments and its variants with
the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary
fall-through markings when it is the case.
[1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=>
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
1148 files changed, 2667 insertions(+), 2737 deletions(-)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma
2020-09-27 17:45 ` Joe Perches
@ 2020-09-27 19:35 ` Julia Lawall
0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2020-09-27 19:35 UTC (permalink / raw)
To: Joe Perches
Cc: Giuseppe Scrivano, Gustavo A. R. Silva, Valdis Klētnieks,
lkp, kernelnewbies, kernel-janitors, LKML, cocci, Andy Whitcroft,
Thomas Gleixner, linux-kernel-mentees, Andrew Morton
On Sun, 27 Sep 2020, Joe Perches wrote:
> On Sun, 2020-09-27 at 19:08 +0200, Julia Lawall wrote:
> > I end up with 208 patches. I'm not sure that sending them all at once
> > would be a good idea...
>
> Last I looked the diffstat for comma -> semicolon was:
>
> 234 files changed, 509 insertions(+), 509 deletions(-)
>
> So it would be nearly 1 patch per individual file,
I have 282 files.
>
> Greg KH does send hundreds of patches for -stable at a time.
>
> So, maybe or maybe not send them all at once.
> Maybe send it in batches of 25 or so.
>
> There's no single right way to do this.
>
> Maybe put up a git tree somewhere and let the
> kernel-robot test compilation.
I compiled all but about 15 and checked those 15 an extra time.
I'll try the small batch approach to get started.
thanks,
julia
> (A nicety might be for the kernel-robot to have some
> option to test pre and post compilation object code
> differences with an optional report)
>
> When I automated 491 patches for /* fallthrough */ to
> fallthrough;, the robot caught a couple problems which
> was great.
>
> https://repo.or.cz/linux-2.6/trivial-mods.git/shortlog/refs/heads/20200310_fallthrough_2
>
> I only posted the first ~30 patches though with
> about 50% acceptance. Gustavo Silva picked up the
> effort and did a great job. Eventually, a single
> treewide patch was posted and accepted by Linus for
> this though after dozens of individual patches went
> through various maintainer trees:
>
> $ git log --shortstat -1 df561f6688fe
> commit df561f6688fef775baa341a0f5d960becd248b11
> Author: Gustavo A. R. Silva <gustavoars@kernel.org>
> Date: Sun Aug 23 17:36:59 2020 -0500
>
> treewide: Use fallthrough pseudo-keyword
>
> Replace the existing /* fall through */ comments and its variants with
> the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary
> fall-through markings when it is the case.
>
> [1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=>
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>
> 1148 files changed, 2667 insertions(+), 2737 deletions(-)
>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-09-27 19:35 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200818184107.f8af232fb58b17160c570874@linux-foundation.org>
[not found] ` <3bf27caf462007dfa75647b040ab3191374a59de.camel@perches.com>
[not found] ` <b0fd63e4b379eda69ee85ab098353582b8c054bb.camel@perches.com>
[not found] ` <alpine.DEB.2.22.394.2008201021400.2524@hadrien>
[not found] ` <a5713d8597065ef986f780499428fcc4cd31c003.camel@perches.com>
[not found] ` <alpine.DEB.2.22.394.2008201856110.2524@hadrien>
2020-08-22 1:08 ` [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma Joe Perches
2020-08-22 3:35 ` Valdis Klētnieks
2020-08-22 5:30 ` Joe Perches
2020-08-22 7:07 ` Julia Lawall
2020-09-24 20:19 ` Thomas Gleixner
2020-09-24 20:21 ` Julia Lawall
2020-09-24 20:33 ` Joe Perches
2020-09-24 21:53 ` Thomas Gleixner
2020-09-24 22:23 ` Joe Perches
2020-09-25 17:06 ` Julia Lawall
2020-09-25 17:26 ` Joe Perches
2020-09-26 19:11 ` Valdis Klētnieks
2020-09-27 17:08 ` Julia Lawall
2020-09-27 17:45 ` Joe Perches
2020-09-27 19:35 ` Julia Lawall
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).