* [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>
---
| 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--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