* [lttng-dev] [PATCH] checkpatch: change status of warning from "BRACES" to "BRACES_SINGLE_STMT" @ 2013-11-13 5:44 Raphaël Beamonte 2013-11-13 14:38 ` Christian Babeux 0 siblings, 1 reply; 4+ messages in thread From: Raphaël Beamonte @ 2013-11-13 5:44 UTC (permalink / raw) The warning about braces that are not necessary for any arm of a statement should be interpreted as a warning about braces around single statements. Signed-off-by: Rapha?l Beamonte <raphael.beamonte at gmail.com> --- extras/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extras/checkpatch.pl b/extras/checkpatch.pl index fdd8484..5a67704 100755 --- a/extras/checkpatch.pl +++ b/extras/checkpatch.pl @@ -3138,7 +3138,7 @@ sub process { $sum_allowed += $_; } if ($sum_allowed == 0) { - WARN("BRACES", + WARN("BRACES_SINGLE_STMT", "braces {} are not necessary for any arm of this statement\n" . $herectx); } elsif ($sum_allowed != $allow && $seen != $allow) { -- 1.7.10.4 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [lttng-dev] [PATCH] checkpatch: change status of warning from "BRACES" to "BRACES_SINGLE_STMT" 2013-11-13 5:44 [lttng-dev] [PATCH] checkpatch: change status of warning from "BRACES" to "BRACES_SINGLE_STMT" Raphaël Beamonte @ 2013-11-13 14:38 ` Christian Babeux 2013-11-13 14:52 ` Raphaël Beamonte 0 siblings, 1 reply; 4+ messages in thread From: Christian Babeux @ 2013-11-13 14:38 UTC (permalink / raw) Hi Rapha?l, Do you have a specific example that is failing for this case of braces detection? I patched checkpatch a while ago in a similar way for braces around single statement blocks, I don't remember the intention behind the particular check you are patching. Thanks, Christian On Wed, Nov 13, 2013 at 12:44 AM, Rapha?l Beamonte <raphael.beamonte at gmail.com> wrote: > The warning about braces that are not necessary for any arm of a statement > should be interpreted as a warning about braces around single statements. > > Signed-off-by: Rapha?l Beamonte <raphael.beamonte at gmail.com> > --- > extras/checkpatch.pl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/extras/checkpatch.pl b/extras/checkpatch.pl > index fdd8484..5a67704 100755 > --- a/extras/checkpatch.pl > +++ b/extras/checkpatch.pl > @@ -3138,7 +3138,7 @@ sub process { > $sum_allowed += $_; > } > if ($sum_allowed == 0) { > - WARN("BRACES", > + WARN("BRACES_SINGLE_STMT", > "braces {} are not necessary for any arm of this statement\n" . $herectx); > } elsif ($sum_allowed != $allow && > $seen != $allow) { > -- > 1.7.10.4 > > > _______________________________________________ > lttng-dev mailing list > lttng-dev at lists.lttng.org > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 4+ messages in thread
* [lttng-dev] [PATCH] checkpatch: change status of warning from "BRACES" to "BRACES_SINGLE_STMT" 2013-11-13 14:38 ` Christian Babeux @ 2013-11-13 14:52 ` Raphaël Beamonte 2013-11-23 22:38 ` Raphaël Beamonte 0 siblings, 1 reply; 4+ messages in thread From: Raphaël Beamonte @ 2013-11-13 14:52 UTC (permalink / raw) Hello Christian, You can try to use checkpatch on the patch I sent yesterday about the expand_path unit test. You'll see that it shows a warning for the following lines : + if (strcmp(valid_tests_inputs[i]. relative_part, ".") == 0) { + relative = cur_path; + } else if (strcmp(valid_tests_inputs[i].relative_part, "..") == 0) { + relative = prev_path; + } else if (strcmp(valid_tests_inputs[i].relative_part, "../..") == 0) { + relative = pprev_path; + } else { + relative = empty; + } As I understood from a discussion with Julien, singles braces are recommended for LTTng source code to limit the number of modified lines in case of change. In this very case, the warning that is shown is not the one about "single statement", but the one about "braces [that] are not necessary for any arm of this statement". It seems that the single statement check you patched is only about single ifs. If we use else or else-if, that's not the same check that is used. Thanks, Rapha?l 2013/11/13 Christian Babeux <christian.babeux at efficios.com>: > Hi Rapha?l, > > Do you have a specific example that is failing for this case of braces > detection? I patched checkpatch a while ago in a similar way for > braces around single statement blocks, I don't remember the intention > behind the particular check you are patching. > > Thanks, > > Christian > > On Wed, Nov 13, 2013 at 12:44 AM, Rapha?l Beamonte > <raphael.beamonte at gmail.com> wrote: >> The warning about braces that are not necessary for any arm of a statement >> should be interpreted as a warning about braces around single statements. >> >> Signed-off-by: Rapha?l Beamonte <raphael.beamonte at gmail.com> >> --- >> extras/checkpatch.pl | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/extras/checkpatch.pl b/extras/checkpatch.pl >> index fdd8484..5a67704 100755 >> --- a/extras/checkpatch.pl >> +++ b/extras/checkpatch.pl >> @@ -3138,7 +3138,7 @@ sub process { >> $sum_allowed += $_; >> } >> if ($sum_allowed == 0) { >> - WARN("BRACES", >> + WARN("BRACES_SINGLE_STMT", >> "braces {} are not necessary for any arm of this statement\n" . $herectx); >> } elsif ($sum_allowed != $allow && >> $seen != $allow) { >> -- >> 1.7.10.4 >> >> >> _______________________________________________ >> lttng-dev mailing list >> lttng-dev at lists.lttng.org >> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 4+ messages in thread
* [lttng-dev] [PATCH] checkpatch: change status of warning from "BRACES" to "BRACES_SINGLE_STMT" 2013-11-13 14:52 ` Raphaël Beamonte @ 2013-11-23 22:38 ` Raphaël Beamonte 0 siblings, 0 replies; 4+ messages in thread From: Raphaël Beamonte @ 2013-11-23 22:38 UTC (permalink / raw) Hello, Do you have some follow-up on this patch ? Thanks, Rapha?l 2013/11/13 Rapha?l Beamonte <raphael.beamonte at gmail.com>: > Hello Christian, > > You can try to use checkpatch on the patch I sent yesterday about the > expand_path unit test. You'll see that it shows a warning for the > following lines : > > + if (strcmp(valid_tests_inputs[i]. > relative_part, ".") == 0) { > + relative = cur_path; > + } else if (strcmp(valid_tests_inputs[i].relative_part, > "..") == 0) { > + relative = prev_path; > + } else if (strcmp(valid_tests_inputs[i].relative_part, > "../..") == 0) { > + relative = pprev_path; > + } else { > + relative = empty; > + } > > As I understood from a discussion with Julien, singles braces are > recommended for LTTng source code to limit the number of modified > lines in case of change. In this very case, the warning that is shown > is not the one about "single statement", but the one about "braces > [that] are not necessary for any arm of this statement". > It seems that the single statement check you patched is only about > single ifs. If we use else or else-if, that's not the same check that > is used. > > Thanks, > Rapha?l > > > > 2013/11/13 Christian Babeux <christian.babeux at efficios.com>: >> Hi Rapha?l, >> >> Do you have a specific example that is failing for this case of braces >> detection? I patched checkpatch a while ago in a similar way for >> braces around single statement blocks, I don't remember the intention >> behind the particular check you are patching. >> >> Thanks, >> >> Christian >> >> On Wed, Nov 13, 2013 at 12:44 AM, Rapha?l Beamonte >> <raphael.beamonte at gmail.com> wrote: >>> The warning about braces that are not necessary for any arm of a statement >>> should be interpreted as a warning about braces around single statements. >>> >>> Signed-off-by: Rapha?l Beamonte <raphael.beamonte at gmail.com> >>> --- >>> extras/checkpatch.pl | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/extras/checkpatch.pl b/extras/checkpatch.pl >>> index fdd8484..5a67704 100755 >>> --- a/extras/checkpatch.pl >>> +++ b/extras/checkpatch.pl >>> @@ -3138,7 +3138,7 @@ sub process { >>> $sum_allowed += $_; >>> } >>> if ($sum_allowed == 0) { >>> - WARN("BRACES", >>> + WARN("BRACES_SINGLE_STMT", >>> "braces {} are not necessary for any arm of this statement\n" . $herectx); >>> } elsif ($sum_allowed != $allow && >>> $seen != $allow) { >>> -- >>> 1.7.10.4 >>> >>> >>> _______________________________________________ >>> lttng-dev mailing list >>> lttng-dev at lists.lttng.org >>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-11-23 22:38 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-11-13 5:44 [lttng-dev] [PATCH] checkpatch: change status of warning from "BRACES" to "BRACES_SINGLE_STMT" Raphaël Beamonte 2013-11-13 14:38 ` Christian Babeux 2013-11-13 14:52 ` Raphaël Beamonte 2013-11-23 22:38 ` Raphaël Beamonte
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox