* 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).