* [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
@ 2020-10-13 6:07 Dwaipayan Ray
2020-10-13 6:16 ` Lukas Bulwahn
0 siblings, 1 reply; 12+ messages in thread
From: Dwaipayan Ray @ 2020-10-13 6:07 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: dwaipayanray1, linux-kernel-mentees
Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
checks.") introduced new checks for author sign off. The format_email
procedure was modified to add comment blocks to the formatted email. But
no space was added between the email address and mail comment.
This causes wrong email formatting in cases where the email is in the form
"author@example.com (Comment block)". The space between the address and
comment block is removed, which causes incorrect parsing of the
formatted email.
An evaluation on checkpatch brought up this case. For example,
on commit 1129d31b55d5 ("ima: Fix ima digest hash table key calculation"),
the following warning was reported:
WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big endian
system concerns)' might be better as 'David.Laight@aculab.com(big endian
system concerns)'
Add a single space in between the address and comment when the
extracted comment is not empty.
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
scripts/checkpatch.pl | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..f1a4e61917eb 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1221,7 +1221,9 @@ sub format_email {
} else {
$formatted_email = "$name$name_comment <$address>";
}
- $formatted_email .= "$comment";
+ if ("$comment" ne "") {
+ $formatted_email .= " $comment";
+ }
return $formatted_email;
}
--
2.27.0
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
2020-10-13 6:07 [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email Dwaipayan Ray
@ 2020-10-13 6:16 ` Lukas Bulwahn
2020-10-13 6:20 ` Dwaipayan Ray
2020-10-14 17:03 ` Aditya
0 siblings, 2 replies; 12+ messages in thread
From: Lukas Bulwahn @ 2020-10-13 6:16 UTC (permalink / raw)
To: Dwaipayan Ray, Aditya Srivastava; +Cc: linux-kernel-mentees
On Tue, 13 Oct 2020, Dwaipayan Ray wrote:
> Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
> checks.") introduced new checks for author sign off. The format_email
> procedure was modified to add comment blocks to the formatted email. But
> no space was added between the email address and mail comment.
>
> This causes wrong email formatting in cases where the email is in the form
> "author@example.com (Comment block)". The space between the address and
> comment block is removed, which causes incorrect parsing of the
> formatted email.
>
> An evaluation on checkpatch brought up this case. For example,
> on commit 1129d31b55d5 ("ima: Fix ima digest hash table key calculation"),
> the following warning was reported:
>
> WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big endian
> system concerns)' might be better as 'David.Laight@aculab.com(big endian
> system concerns)'
>
> Add a single space in between the address and comment when the
> extracted comment is not empty.
>
Dwaipayan, looks good to me.
So, how about a 'Fixes:' tag?
Aditya, can you rerun your evaluation with this fix patch applied on top?
Then, we need a comparison for:
1. completely before vs. after the two patches, and
2. after the first patch vs. after the two patches (to see that the fix
works)
More support on evaluation from others interested to become mentees are of
course welcome.
Lukas
> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> ---
> scripts/checkpatch.pl | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fab38b493cef..f1a4e61917eb 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1221,7 +1221,9 @@ sub format_email {
> } else {
> $formatted_email = "$name$name_comment <$address>";
> }
> - $formatted_email .= "$comment";
> + if ("$comment" ne "") {
> + $formatted_email .= " $comment";
> + }
> return $formatted_email;
> }
>
> --
> 2.27.0
>
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
2020-10-13 6:16 ` Lukas Bulwahn
@ 2020-10-13 6:20 ` Dwaipayan Ray
2020-10-13 6:29 ` Lukas Bulwahn
2020-10-14 17:03 ` Aditya
1 sibling, 1 reply; 12+ messages in thread
From: Dwaipayan Ray @ 2020-10-13 6:20 UTC (permalink / raw)
To: Lukas Bulwahn; +Cc: linux-kernel-mentees, Aditya Srivastava
On Tue, Oct 13, 2020 at 11:46 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
>
>
>
> On Tue, 13 Oct 2020, Dwaipayan Ray wrote:
>
> > Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
> > checks.") introduced new checks for author sign off. The format_email
> > procedure was modified to add comment blocks to the formatted email. But
> > no space was added between the email address and mail comment.
> >
> > This causes wrong email formatting in cases where the email is in the form
> > "author@example.com (Comment block)". The space between the address and
> > comment block is removed, which causes incorrect parsing of the
> > formatted email.
> >
> > An evaluation on checkpatch brought up this case. For example,
> > on commit 1129d31b55d5 ("ima: Fix ima digest hash table key calculation"),
> > the following warning was reported:
> >
> > WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big endian
> > system concerns)' might be better as 'David.Laight@aculab.com(big endian
> > system concerns)'
> >
> > Add a single space in between the address and comment when the
> > extracted comment is not empty.
> >
>
> Dwaipayan, looks good to me.
>
> So, how about a 'Fixes:' tag?
>
> Aditya, can you rerun your evaluation with this fix patch applied on top?
>
> Then, we need a comparison for:
> 1. completely before vs. after the two patches, and
> 2. after the first patch vs. after the two patches (to see that the fix
> works)
>
> More support on evaluation from others interested to become mentees are of
> course welcome.
>
> Lukas
Sure I will add the tag.
Do I send it to LKML after this or after more evaluation?
I did run on some of the newly generated bad sign off warnings and
they seem to be fixed.
Thanks,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
2020-10-13 6:20 ` Dwaipayan Ray
@ 2020-10-13 6:29 ` Lukas Bulwahn
0 siblings, 0 replies; 12+ messages in thread
From: Lukas Bulwahn @ 2020-10-13 6:29 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, Aditya Srivastava
On Tue, 13 Oct 2020, Dwaipayan Ray wrote:
> On Tue, Oct 13, 2020 at 11:46 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
> >
> >
> >
> > On Tue, 13 Oct 2020, Dwaipayan Ray wrote:
> >
> > > Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
> > > checks.") introduced new checks for author sign off. The format_email
> > > procedure was modified to add comment blocks to the formatted email. But
> > > no space was added between the email address and mail comment.
> > >
> > > This causes wrong email formatting in cases where the email is in the form
> > > "author@example.com (Comment block)". The space between the address and
> > > comment block is removed, which causes incorrect parsing of the
> > > formatted email.
> > >
> > > An evaluation on checkpatch brought up this case. For example,
> > > on commit 1129d31b55d5 ("ima: Fix ima digest hash table key calculation"),
> > > the following warning was reported:
> > >
> > > WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big endian
> > > system concerns)' might be better as 'David.Laight@aculab.com(big endian
> > > system concerns)'
> > >
> > > Add a single space in between the address and comment when the
> > > extracted comment is not empty.
> > >
> >
> > Dwaipayan, looks good to me.
> >
> > So, how about a 'Fixes:' tag?
> >
> > Aditya, can you rerun your evaluation with this fix patch applied on top?
> >
> > Then, we need a comparison for:
> > 1. completely before vs. after the two patches, and
> > 2. after the first patch vs. after the two patches (to see that the fix
> > works)
> >
> > More support on evaluation from others interested to become mentees are of
> > course welcome.
> >
> > Lukas
>
> Sure I will add the tag.
> Do I send it to LKML after this or after more evaluation?
> I did run on some of the newly generated bad sign off warnings and
> they seem to be fixed.
>
Go send it out; we will run the evaluation just as sanity checking...
Remember that currently the merge window of the next release is ongoing;
so, Joe will probably review and ack, but Andrew Morton might only pick it
up in a few weeks (after the hot phase of the merge window).
Maybe a fix will be picked up faster though. We will see.
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
2020-10-13 6:16 ` Lukas Bulwahn
2020-10-13 6:20 ` Dwaipayan Ray
@ 2020-10-14 17:03 ` Aditya
2020-10-17 5:38 ` Aditya
1 sibling, 1 reply; 12+ messages in thread
From: Aditya @ 2020-10-14 17:03 UTC (permalink / raw)
To: Lukas Bulwahn, Dwaipayan Ray; +Cc: linux-kernel-mentees
On 13/10/20 11:46 am, Lukas Bulwahn wrote:
>
> On Tue, 13 Oct 2020, Dwaipayan Ray wrote:
>
>> Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
>> checks.") introduced new checks for author sign off. The format_email
>> procedure was modified to add comment blocks to the formatted email. But
>> no space was added between the email address and mail comment.
>>
>> This causes wrong email formatting in cases where the email is in the form
>> "author@example.com (Comment block)". The space between the address and
>> comment block is removed, which causes incorrect parsing of the
>> formatted email.
>>
>> An evaluation on checkpatch brought up this case. For example,
>> on commit 1129d31b55d5 ("ima: Fix ima digest hash table key calculation"),
>> the following warning was reported:
>>
>> WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big endian
>> system concerns)' might be better as 'David.Laight@aculab.com(big endian
>> system concerns)'
>>
>> Add a single space in between the address and comment when the
>> extracted comment is not empty.
>>
> Dwaipayan, looks good to me.
>
> So, how about a 'Fixes:' tag?
>
> Aditya, can you rerun your evaluation with this fix patch applied on top?
>
> Then, we need a comparison for:
> 1. completely before vs. after the two patches, and
> 2. after the first patch vs. after the two patches (to see that the fix
> works)
>
> More support on evaluation from others interested to become mentees are of
> course welcome.
>
> Lukas
>
>> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
>> ---
>> scripts/checkpatch.pl | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index fab38b493cef..f1a4e61917eb 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -1221,7 +1221,9 @@ sub format_email {
>> } else {
>> $formatted_email = "$name$name_comment <$address>";
>> }
>> - $formatted_email .= "$comment";
>> + if ("$comment" ne "") {
>> + $formatted_email .= " $comment";
>> + }
>> return $formatted_email;
>> }
>>
>> --
>> 2.27.0
>>
>>
Hi Sir
I have analyzed the reports using this patch and made comparison. The links are as follows:
1. completely before vs. after this patch: https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task2/reports/analysis/relative_change/before_v_second/summary_relative.txt
2. after the first patch vs. after the two patches: https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task2/reports/analysis/relative_change/first_v_second/summary_relative.txt
Kindly let me know if you have any questions.
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
2020-10-14 17:03 ` Aditya
@ 2020-10-17 5:38 ` Aditya
2020-10-18 7:43 ` Lukas Bulwahn
0 siblings, 1 reply; 12+ messages in thread
From: Aditya @ 2020-10-17 5:38 UTC (permalink / raw)
To: Lukas Bulwahn; +Cc: linux-kernel-mentees
On Wed, Oct 14, 2020 at 10:33 PM Aditya <yashsri421@gmail.com> wrote:
>
> On 13/10/20 11:46 am, Lukas Bulwahn wrote:
> >
> > On Tue, 13 Oct 2020, Dwaipayan Ray wrote:
> >
> >> Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
> >> checks.") introduced new checks for author sign off. The format_email
> >> procedure was modified to add comment blocks to the formatted email. But
> >> no space was added between the email address and mail comment.
> >>
> >> This causes wrong email formatting in cases where the email is in the
> form
> >> "author@example.com (Comment block)". The space between the address and
> >> comment block is removed, which causes incorrect parsing of the
> >> formatted email.
> >>
> >> An evaluation on checkpatch brought up this case. For example,
> >> on commit 1129d31b55d5 ("ima: Fix ima digest hash table key
> calculation"),
> >> the following warning was reported:
> >>
> >> WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big
> endian
> >> system concerns)' might be better as 'David.Laight@aculab.com(big
> endian
> >> system concerns)'
> >>
> >> Add a single space in between the address and comment when the
> >> extracted comment is not empty.
> >>
> > Dwaipayan, looks good to me.
> >
> > So, how about a 'Fixes:' tag?
> >
> > Aditya, can you rerun your evaluation with this fix patch applied on top?
> >
> > Then, we need a comparison for:
> > 1. completely before vs. after the two patches, and
> > 2. after the first patch vs. after the two patches (to see that the fix
> > works)
> >
> > More support on evaluation from others interested to become mentees are
> of
> > course welcome.
> >
> > Lukas
> >
> >> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> >> ---
> >> scripts/checkpatch.pl | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> >> index fab38b493cef..f1a4e61917eb 100755
> >> --- a/scripts/checkpatch.pl
> >> +++ b/scripts/checkpatch.pl
> >> @@ -1221,7 +1221,9 @@ sub format_email {
> >> } else {
> >> $formatted_email = "$name$name_comment <$address>";
> >> }
> >> - $formatted_email .= "$comment";
> >> + if ("$comment" ne "") {
> >> + $formatted_email .= " $comment";
> >> + }
> >> return $formatted_email;
> >> }
> >>
> >> --
> >> 2.27.0
> >>
> >>
> Hi Sir
>
> I have analyzed the reports using this patch and made comparison. The
> links are as follows:
>
> 1. completely before vs. after this patch:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task2/reports/analysis/relative_change/before_v_second/summary_relative.txt
> 2. after the first patch vs. after the two patches:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task2/reports/analysis/relative_change/first_v_second/summary_relative.txt
>
> Kindly let me know if you have any questions.
>
> Aditya
>
>
Hi Sir
I have completed all the assigned tasks. What should be my next steps to participate as a mentee in the community bridge program?
Kindly guide me regarding it.
Thank You
Sincerely
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
2020-10-17 5:38 ` Aditya
@ 2020-10-18 7:43 ` Lukas Bulwahn
2020-10-19 9:26 ` Aditya
0 siblings, 1 reply; 12+ messages in thread
From: Lukas Bulwahn @ 2020-10-18 7:43 UTC (permalink / raw)
To: Aditya; +Cc: linux-kernel-mentees
On Sat, 17 Oct 2020, Aditya wrote:
>
> On Wed, Oct 14, 2020 at 10:33 PM Aditya <yashsri421@gmail.com> wrote:
>
> >
> > On 13/10/20 11:46 am, Lukas Bulwahn wrote:
> > >
> > > On Tue, 13 Oct 2020, Dwaipayan Ray wrote:
> > >
> > >> Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
> > >> checks.") introduced new checks for author sign off. The format_email
> > >> procedure was modified to add comment blocks to the formatted email. But
> > >> no space was added between the email address and mail comment.
> > >>
> > >> This causes wrong email formatting in cases where the email is in the
> > form
> > >> "author@example.com (Comment block)". The space between the address and
> > >> comment block is removed, which causes incorrect parsing of the
> > >> formatted email.
> > >>
> > >> An evaluation on checkpatch brought up this case. For example,
> > >> on commit 1129d31b55d5 ("ima: Fix ima digest hash table key
> > calculation"),
> > >> the following warning was reported:
> > >>
> > >> WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big
> > endian
> > >> system concerns)' might be better as 'David.Laight@aculab.com(big
> > endian
> > >> system concerns)'
> > >>
> > >> Add a single space in between the address and comment when the
> > >> extracted comment is not empty.
> > >>
> > > Dwaipayan, looks good to me.
> > >
> > > So, how about a 'Fixes:' tag?
> > >
> > > Aditya, can you rerun your evaluation with this fix patch applied on top?
> > >
> > > Then, we need a comparison for:
> > > 1. completely before vs. after the two patches, and
> > > 2. after the first patch vs. after the two patches (to see that the fix
> > > works)
> > >
> > > More support on evaluation from others interested to become mentees are
> > of
> > > course welcome.
> > >
> > > Lukas
> > >
> > >> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> > >> ---
> > >> scripts/checkpatch.pl | 4 +++-
> > >> 1 file changed, 3 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > >> index fab38b493cef..f1a4e61917eb 100755
> > >> --- a/scripts/checkpatch.pl
> > >> +++ b/scripts/checkpatch.pl
> > >> @@ -1221,7 +1221,9 @@ sub format_email {
> > >> } else {
> > >> $formatted_email = "$name$name_comment <$address>";
> > >> }
> > >> - $formatted_email .= "$comment";
> > >> + if ("$comment" ne "") {
> > >> + $formatted_email .= " $comment";
> > >> + }
> > >> return $formatted_email;
> > >> }
> > >>
> > >> --
> > >> 2.27.0
> > >>
> > >>
> > Hi Sir
> >
> > I have analyzed the reports using this patch and made comparison. The
> > links are as follows:
> >
> > 1. completely before vs. after this patch:
> > https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task2/reports/analysis/relative_change/before_v_second/summary_relative.txt
> > 2. after the first patch vs. after the two patches:
> > https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task2/reports/analysis/relative_change/first_v_second/summary_relative.txt
> >
> > Kindly let me know if you have any questions.
> >
> > Aditya
> >
> >
>
> Hi Sir
> I have completed all the assigned tasks. What should be my next steps to participate as a mentee in the community bridge program?
> Kindly guide me regarding it.
>
(your email client is broken; it should not spread lines so long...)
Aditya, you are on a good way to be towards acceptance for a kernel
mentorship.
If I am not mistaken, you have not created yet a non-trivial patch to
checkpatch.pl and got your patch accepted.
Look at your own checkpatch.pl evaluation, find a relevant case where
checkpatch.pl reports a large class of false positives and think about how
to improve checkpatch.pl.
Especially, when looking at checkpatch.pl complaining about suspicious
code patterns or style and formatting, you might want to compare to the
capabilities of compiler static analysers (clang-tidy) or clang-format.
Improving those tools, making them the warning-free for the specific
class and supporting to make them default tools might also be a good
extension to the goals of checkpatch.
Let us know which kind of false positive class you found and would like to
improve.
Dwaipayan and I can help if you really do not find anything at all...
Good luck :)
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
2020-10-18 7:43 ` Lukas Bulwahn
@ 2020-10-19 9:26 ` Aditya
2020-10-19 10:42 ` Lukas Bulwahn
0 siblings, 1 reply; 12+ messages in thread
From: Aditya @ 2020-10-19 9:26 UTC (permalink / raw)
To: Lukas Bulwahn; +Cc: linux-kernel-mentees
On 18/10/20 1:13 pm, Lukas Bulwahn wrote:
>
>
> On Sat, 17 Oct 2020, Aditya wrote:
>
>>
>> On Wed, Oct 14, 2020 at 10:33 PM Aditya <yashsri421@gmail.com> wrote:
>>
>>>
>>> On 13/10/20 11:46 am, Lukas Bulwahn wrote:
>>>>
>>>> On Tue, 13 Oct 2020, Dwaipayan Ray wrote:
>>>>
>>>>> Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
>>>>> checks.") introduced new checks for author sign off. The format_email
>>>>> procedure was modified to add comment blocks to the formatted email. But
>>>>> no space was added between the email address and mail comment.
>>>>>
>>>>> This causes wrong email formatting in cases where the email is in the
>>> form
>>>>> "author@example.com (Comment block)". The space between the address and
>>>>> comment block is removed, which causes incorrect parsing of the
>>>>> formatted email.
>>>>>
>>>>> An evaluation on checkpatch brought up this case. For example,
>>>>> on commit 1129d31b55d5 ("ima: Fix ima digest hash table key
>>> calculation"),
>>>>> the following warning was reported:
>>>>>
>>>>> WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big
>>> endian
>>>>> system concerns)' might be better as 'David.Laight@aculab.com(big
>>> endian
>>>>> system concerns)'
>>>>>
>>>>> Add a single space in between the address and comment when the
>>>>> extracted comment is not empty.
>>>>>
>>>> Dwaipayan, looks good to me.
>>>>
>>>> So, how about a 'Fixes:' tag?
>>>>
>>>> Aditya, can you rerun your evaluation with this fix patch applied on top?
>>>>
>>>> Then, we need a comparison for:
>>>> 1. completely before vs. after the two patches, and
>>>> 2. after the first patch vs. after the two patches (to see that the fix
>>>> works)
>>>>
>>>> More support on evaluation from others interested to become mentees are
>>> of
>>>> course welcome.
>>>>
>>>> Lukas
>>>>
>>>>> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
>>>>> ---
>>>>> scripts/checkpatch.pl | 4 +++-
>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>>>> index fab38b493cef..f1a4e61917eb 100755
>>>>> --- a/scripts/checkpatch.pl
>>>>> +++ b/scripts/checkpatch.pl
>>>>> @@ -1221,7 +1221,9 @@ sub format_email {
>>>>> } else {
>>>>> $formatted_email = "$name$name_comment <$address>";
>>>>> }
>>>>> - $formatted_email .= "$comment";
>>>>> + if ("$comment" ne "") {
>>>>> + $formatted_email .= " $comment";
>>>>> + }
>>>>> return $formatted_email;
>>>>> }
>>>>>
>>>>> --
>>>>> 2.27.0
>>>>>
>>>>>
>>> Hi Sir
>>>
>>> I have analyzed the reports using this patch and made comparison. The
>>> links are as follows:
>>>
>>> 1. completely before vs. after this patch:
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task2/reports/analysis/relative_change/before_v_second/summary_relative.txt
>>> 2. after the first patch vs. after the two patches:
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task2/reports/analysis/relative_change/first_v_second/summary_relative.txt
>>>
>>> Kindly let me know if you have any questions.
>>>
>>> Aditya
>>>
>>>
>>
>> Hi Sir
>> I have completed all the assigned tasks. What should be my next steps to participate as a mentee in the community bridge program?
>> Kindly guide me regarding it.
>>
>
> (your email client is broken; it should not spread lines so long...)
>
> Aditya, you are on a good way to be towards acceptance for a kernel
> mentorship.
>
> If I am not mistaken, you have not created yet a non-trivial patch to
> checkpatch.pl and got your patch accepted.
>
>
> Look at your own checkpatch.pl evaluation, find a relevant case where
> checkpatch.pl reports a large class of false positives and think about how
> to improve checkpatch.pl.
>
> Especially, when looking at checkpatch.pl complaining about suspicious
> code patterns or style and formatting, you might want to compare to the
> capabilities of compiler static analysers (clang-tidy) or clang-format.
>
> Improving those tools, making them the warning-free for the specific
> class and supporting to make them default tools might also be a good
> extension to the goals of checkpatch.
>
> Let us know which kind of false positive class you found and would like to
> improve.
>
> Dwaipayan and I can help if you really do not find anything at all...
>
> Good luck :)
>
>
> Lukas
>
Hi Sir,
I observed the reports as you suggested.
I found out that "REPEATED_WORD" is the third most frequent warning in
v6..v8 with frequency of 2797. Although this does give correct
messages most of the times, but a large proportion(probably more than
50%) of this error comes out of hexadecimal being a part of the commit
or code.
For eg, WARNING:REPEATED_WORD: Possible repeated word: 'ff'
#21:
Code: 00 77 27 48 81 ff 00 00 01 00 76 07 0f b7 d7 ed 0f c8 c3 55 48
c7 c6 3b ee d5 9f 48 89 e5 e8 67 fc ff ff b8 ff ff ff ff 5d c3 <8b> 07
0f c8 c3 66 66 2e 0f 1f 84 00 00 00 00 00 48 81 fe ff ff 03
Now, here it reports 'ff' to be repeated many times. But I believe
that this was not the expected outcome from this warning.
I think that such warnings should be avoided from this class tag of
warning.
My approach: Use regex to know if the repeated words are of '2' length
and occur among such pattern which consists of 2 lettered words
repeated 5-6 times,ie. size(/[a-f0-9]{2}\s/)>6, eg. "04 19 00 00 c3 48
83 ec 08 e8 0a fa ff ff 48 89 04 24 b8 02 00 00 00 0f".
As normally such occurrences do not occur in English, it can be an
effective method to avoid such hexadecimal occurrences.
Kindly let me know if I can proceed with this. Also, please suggest if
there is a better way to counter this issue.
Thank You
Sincerely
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
2020-10-19 9:26 ` Aditya
@ 2020-10-19 10:42 ` Lukas Bulwahn
0 siblings, 0 replies; 12+ messages in thread
From: Lukas Bulwahn @ 2020-10-19 10:42 UTC (permalink / raw)
To: Aditya; +Cc: linux-kernel-mentees
On Mon, 19 Oct 2020, Aditya wrote:
> On 18/10/20 1:13 pm, Lukas Bulwahn wrote:
> >
> >
> > On Sat, 17 Oct 2020, Aditya wrote:
> >
> >>
> >> On Wed, Oct 14, 2020 at 10:33 PM Aditya <yashsri421@gmail.com> wrote:
> >>
> >>>
> >>> On 13/10/20 11:46 am, Lukas Bulwahn wrote:
> >>>>
> >>>> On Tue, 13 Oct 2020, Dwaipayan Ray wrote:
> >>>>
> >>>>> Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
> >>>>> checks.") introduced new checks for author sign off. The format_email
> >>>>> procedure was modified to add comment blocks to the formatted email. But
> >>>>> no space was added between the email address and mail comment.
> >>>>>
> >>>>> This causes wrong email formatting in cases where the email is in the
> >>> form
> >>>>> "author@example.com (Comment block)". The space between the address and
> >>>>> comment block is removed, which causes incorrect parsing of the
> >>>>> formatted email.
> >>>>>
> >>>>> An evaluation on checkpatch brought up this case. For example,
> >>>>> on commit 1129d31b55d5 ("ima: Fix ima digest hash table key
> >>> calculation"),
> >>>>> the following warning was reported:
> >>>>>
> >>>>> WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big
> >>> endian
> >>>>> system concerns)' might be better as 'David.Laight@aculab.com(big
> >>> endian
> >>>>> system concerns)'
> >>>>>
> >>>>> Add a single space in between the address and comment when the
> >>>>> extracted comment is not empty.
> >>>>>
> >>>> Dwaipayan, looks good to me.
> >>>>
> >>>> So, how about a 'Fixes:' tag?
> >>>>
> >>>> Aditya, can you rerun your evaluation with this fix patch applied on top?
> >>>>
> >>>> Then, we need a comparison for:
> >>>> 1. completely before vs. after the two patches, and
> >>>> 2. after the first patch vs. after the two patches (to see that the fix
> >>>> works)
> >>>>
> >>>> More support on evaluation from others interested to become mentees are
> >>> of
> >>>> course welcome.
> >>>>
> >>>> Lukas
> >>>>
> >>>>> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> >>>>> ---
> >>>>> scripts/checkpatch.pl | 4 +++-
> >>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> >>>>> index fab38b493cef..f1a4e61917eb 100755
> >>>>> --- a/scripts/checkpatch.pl
> >>>>> +++ b/scripts/checkpatch.pl
> >>>>> @@ -1221,7 +1221,9 @@ sub format_email {
> >>>>> } else {
> >>>>> $formatted_email = "$name$name_comment <$address>";
> >>>>> }
> >>>>> - $formatted_email .= "$comment";
> >>>>> + if ("$comment" ne "") {
> >>>>> + $formatted_email .= " $comment";
> >>>>> + }
> >>>>> return $formatted_email;
> >>>>> }
> >>>>>
> >>>>> --
> >>>>> 2.27.0
> >>>>>
> >>>>>
> >>> Hi Sir
> >>>
> >>> I have analyzed the reports using this patch and made comparison. The
> >>> links are as follows:
> >>>
> >>> 1. completely before vs. after this patch:
> >>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task2/reports/analysis/relative_change/before_v_second/summary_relative.txt
> >>> 2. after the first patch vs. after the two patches:
> >>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task2/reports/analysis/relative_change/first_v_second/summary_relative.txt
> >>>
> >>> Kindly let me know if you have any questions.
> >>>
> >>> Aditya
> >>>
> >>>
> >>
> >> Hi Sir
> >> I have completed all the assigned tasks. What should be my next steps to participate as a mentee in the community bridge program?
> >> Kindly guide me regarding it.
> >>
> >
> > (your email client is broken; it should not spread lines so long...)
> >
> > Aditya, you are on a good way to be towards acceptance for a kernel
> > mentorship.
> >
> > If I am not mistaken, you have not created yet a non-trivial patch to
> > checkpatch.pl and got your patch accepted.
> >
> >
> > Look at your own checkpatch.pl evaluation, find a relevant case where
> > checkpatch.pl reports a large class of false positives and think about how
> > to improve checkpatch.pl.
> >
> > Especially, when looking at checkpatch.pl complaining about suspicious
> > code patterns or style and formatting, you might want to compare to the
> > capabilities of compiler static analysers (clang-tidy) or clang-format.
> >
> > Improving those tools, making them the warning-free for the specific
> > class and supporting to make them default tools might also be a good
> > extension to the goals of checkpatch.
> >
> > Let us know which kind of false positive class you found and would like to
> > improve.
> >
> > Dwaipayan and I can help if you really do not find anything at all...
> >
> > Good luck :)
> >
> >
> > Lukas
> >
>
> Hi Sir,
> I observed the reports as you suggested.
> I found out that "REPEATED_WORD" is the third most frequent warning in
> v6..v8 with frequency of 2797. Although this does give correct
> messages most of the times, but a large proportion(probably more than
> 50%) of this error comes out of hexadecimal being a part of the commit
> or code.
> For eg, WARNING:REPEATED_WORD: Possible repeated word: 'ff'
> #21:
> Code: 00 77 27 48 81 ff 00 00 01 00 76 07 0f b7 d7 ed 0f c8 c3 55 48
> c7 c6 3b ee d5 9f 48 89 e5 e8 67 fc ff ff b8 ff ff ff ff 5d c3 <8b> 07
> 0f c8 c3 66 66 2e 0f 1f 84 00 00 00 00 00 48 81 fe ff ff 03
>
> Now, here it reports 'ff' to be repeated many times. But I believe
> that this was not the expected outcome from this warning.
>
> I think that such warnings should be avoided from this class tag of
> warning.
>
> My approach: Use regex to know if the repeated words are of '2' length
> and occur among such pattern which consists of 2 lettered words
> repeated 5-6 times,ie. size(/[a-f0-9]{2}\s/)>6, eg. "04 19 00 00 c3 48
> 83 ec 08 e8 0a fa ff ff 48 89 04 24 b8 02 00 00 00 0f".
> As normally such occurrences do not occur in English, it can be an
> effective method to avoid such hexadecimal occurrences.
>
> Kindly let me know if I can proceed with this. Also, please suggest if
> there is a better way to counter this issue.
>
Sounds good to me.
- Create a patch that addresses the issue.
- Write a good commit message according to the kernel recommendations.
- Share the patch with this list.
- We will then review here first.
- Show the impact of your patch with an evaluation of before and after the
patch (apply it to linux-next and with Dwaipayan's pending patches on this
topic).
When we reviewed it, you can send the patch to the larger group, Joe
Perches and lkml.
Looking forward to your patch.
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
2020-10-13 6:57 ` Joe Perches
@ 2020-10-13 7:35 ` Dwaipayan Ray
0 siblings, 0 replies; 12+ messages in thread
From: Dwaipayan Ray @ 2020-10-13 7:35 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel
On Tue, Oct 13, 2020 at 12:27 PM Joe Perches <joe@perches.com> wrote:
>
> On Tue, 2020-10-13 at 12:06 +0530, Dwaipayan Ray wrote:
> > Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
> > checks.") introduced new checks for author sign off. The format_email
> > procedure was modified to add comment blocks to the formatted email. But
> > no space was added between the email address and mail comment.
> >
> > This causes wrong email formatting in cases where the email is in the form
> > "author@example.com (Comment block)". The space between the address and
> > comment block is removed, which causes incorrect parsing of the
> > formatted email.
> >
> > An evaluation on checkpatch brought up this case. For example,
> > on commit 1129d31b55d5 ("ima: Fix ima digest hash table key calculation"),
> > the following warning was reported:
> >
> > WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big endian
> > system concerns)' might be better as 'David.Laight@aculab.com(big endian
> > system concerns)'
>
> Strictly, a comment or multiple comments can exist
> in any part of an email
>
> "John (Jon) Smith (Smitty) <jsmith@domain.tld> (tld is the best tld)
>
> > Add a single space in between the address and comment when the
> > extracted comment is not empty.
>
> So after the address is not necessarily the best
> location for a comment.
>
>
Yes I agree and in cases like that the check works perfectly.
But now consider the case when the name is empty:
Like:
jsmith@domain.tld (tld is the best tld).
In this case when the formatted email is generated, we have
the following: "jsmith@domain.tld(tld is the best tld)".
If this email is parsed again, the entire part is parsed as the email address,
which causes a mismatch later on.
It is for only this case that an extra white space is needed for the
parsing, probably because of how the comment is captured, for emails
with empty names:
$formatted_email =~ /(\S+\@\S+)(.*)$/
So it depends on the whitespace after address to demarcate the comment.
Some more examples of what I intend to solve:
'<stable@vger.kernel.org> [5.3+]'
'<stable@vger.kernel.org> # v5.7+'
'David.Laight@aculab.com (big endian system concerns)'
Thanks,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
2020-10-13 6:36 Dwaipayan Ray
@ 2020-10-13 6:57 ` Joe Perches
2020-10-13 7:35 ` Dwaipayan Ray
0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2020-10-13 6:57 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel
On Tue, 2020-10-13 at 12:06 +0530, Dwaipayan Ray wrote:
> Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
> checks.") introduced new checks for author sign off. The format_email
> procedure was modified to add comment blocks to the formatted email. But
> no space was added between the email address and mail comment.
>
> This causes wrong email formatting in cases where the email is in the form
> "author@example.com (Comment block)". The space between the address and
> comment block is removed, which causes incorrect parsing of the
> formatted email.
>
> An evaluation on checkpatch brought up this case. For example,
> on commit 1129d31b55d5 ("ima: Fix ima digest hash table key calculation"),
> the following warning was reported:
>
> WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big endian
> system concerns)' might be better as 'David.Laight@aculab.com(big endian
> system concerns)'
Strictly, a comment or multiple comments can exist
in any part of an email
"John (Jon) Smith (Smitty) <jsmith@domain.tld> (tld is the best tld)
> Add a single space in between the address and comment when the
> extracted comment is not empty.
So after the address is not necessarily the best
location for a comment.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email
@ 2020-10-13 6:36 Dwaipayan Ray
2020-10-13 6:57 ` Joe Perches
0 siblings, 1 reply; 12+ messages in thread
From: Dwaipayan Ray @ 2020-10-13 6:36 UTC (permalink / raw)
To: joe; +Cc: dwaipayanray1, linux-kernel-mentees, linux-kernel
Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
checks.") introduced new checks for author sign off. The format_email
procedure was modified to add comment blocks to the formatted email. But
no space was added between the email address and mail comment.
This causes wrong email formatting in cases where the email is in the form
"author@example.com (Comment block)". The space between the address and
comment block is removed, which causes incorrect parsing of the
formatted email.
An evaluation on checkpatch brought up this case. For example,
on commit 1129d31b55d5 ("ima: Fix ima digest hash table key calculation"),
the following warning was reported:
WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big endian
system concerns)' might be better as 'David.Laight@aculab.com(big endian
system concerns)'
Add a single space in between the address and comment when the
extracted comment is not empty.
Fixes: 0c01921e56f9 ("checkpatch: add new warnings to author signoff checks.")
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
scripts/checkpatch.pl | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..f1a4e61917eb 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1221,7 +1221,9 @@ sub format_email {
} else {
$formatted_email = "$name$name_comment <$address>";
}
- $formatted_email .= "$comment";
+ if ("$comment" ne "") {
+ $formatted_email .= " $comment";
+ }
return $formatted_email;
}
--
2.27.0
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-10-19 10:42 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-13 6:07 [Linux-kernel-mentees] [PATCH v2] checkpatch: fix missing whitespace in formatted email Dwaipayan Ray
2020-10-13 6:16 ` Lukas Bulwahn
2020-10-13 6:20 ` Dwaipayan Ray
2020-10-13 6:29 ` Lukas Bulwahn
2020-10-14 17:03 ` Aditya
2020-10-17 5:38 ` Aditya
2020-10-18 7:43 ` Lukas Bulwahn
2020-10-19 9:26 ` Aditya
2020-10-19 10:42 ` Lukas Bulwahn
2020-10-13 6:36 Dwaipayan Ray
2020-10-13 6:57 ` Joe Perches
2020-10-13 7:35 ` 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).