On Mon, 19 Oct 2020, Aditya wrote:

On 18/10/20 1:13 pm, Lukas Bulwahn wrote:

quoted

On Sat, 17 Oct 2020, Aditya wrote:

quoted

On Wed, Oct 14, 2020 at 10:33 PM Aditya [off-list ref] wrote:

quoted

On 13/10/20 11:46 am, Lukas Bulwahn wrote:

quoted

On Tue, 13 Oct 2020, Dwaipayan Ray wrote:

quoted

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

quoted

quoted

"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"),

quoted

quoted

the following warning was reported:

WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big 

endian

quoted

quoted

system concerns)' might be better as 'David.Laight@aculab.com(big 

endian

quoted

quoted

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 

quoted

course welcome.

Lukas
 

quoted

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