On 11/11/20 12:13 am, Lukas Bulwahn wrote:

On Di., 10. Nov. 2020 at 19:13, Aditya [off-list ref] wrote:

quoted

On 10/11/20 8:52 pm, Lukas Bulwahn wrote:

quoted

On Tue, Nov 10, 2020 at 4:13 PM Aditya [off-list ref] wrote:

quoted

On 10/11/20 7:30 pm, Lukas Bulwahn wrote:

quoted

On Tue, Nov 10, 2020 at 2:06 PM Aditya Srivastava <

yashsri421@gmail.com> wrote:

quoted

quoted

quoted

quoted

Currently checkpatch warns us if there is no 'Signed-off-by' line
for the patch.

E.g., running checkpatch on commit 9ac060a708e0 ("leaking_addresses:
Completely remove --version flag") reports this error:

ERROR: Missing Signed-off-by: line(s)

Provide a fix by adding a Signed-off-by line corresponding to the

author

quoted

quoted

quoted

quoted

of the patch before the patch separator line. Also avoid this error

for

quoted

quoted

quoted

quoted

the commits where some typo is present in the sign off.

E.g. for commit 8cde5d5f7361 ("bus: ti-sysc: Detect omap4 type timers
for quirk") we get missing sign off as well as bad sign off for:

Siganed-off-by: Tony Lindgren [off-list ref]

Here it is probably best to give BAD_SIGN_OFF warning for Non-standard
signature and avoid MISSING_SIGN_OFF

Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Aditya Srivastava <redacted>
---
This patch was made after applying

https://lore.kernel.org/linux-kernel-mentees/20201108100632.75340-1-dwaipayanray1@gmail.com/ (local)

quoted

quoted

quoted

quoted

and my last changes at

https://lore.kernel.org/linux-kernel-mentees/20201108134317.25400-1-yashsri421@gmail.com/ (local)

quoted

quoted

quoted

quoted

 scripts/checkpatch.pl | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index cb46288127ac..2deffd0c091b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl

@@ -2404,6 +2404,8 @@ sub process {

        my $last_blank_line = 0;
        my $last_coalesced_string_linenr = -1;
+       my $patch_separator_linenr = 0;
+       my $non_standard_signature = 0;

        our @report = ();
        our $cnt_lines = 0;

@@ -2755,6 +2757,10 @@ sub process {
                if ($line =~ /^---$/) {
                        $has_patch_separator = 1;
                        $in_commit_log = 0;
+                       # to add missing sign off line before diff(s)
+                       if($patch_separator_linenr == 0) {
+                               $patch_separator_linenr = $linenr;

Well are you sure linenr is correct? (As I wrote, I do not know, but
you should know.)

I think you would need to create a patch for testing your feature
where multiple things are fixed within the same patch and then check
if the fix is added at the right place.

I checked the fix over following patch.

I have modified the patch such that it has co-developed-by as the
author(nominal patch author warning, which requires this line to be
deleted)

Also 'occuring', which is a typo(where the typo needs to be replaced)

It works as expected.

Well to really check if it works as expected, you need to have a test
patch where a fix adds multiple lines into the fixed patch before the
line you try to record.

Then, fixlinenr and linenr are probably more than just 1 line apart,
but really multiple lines apart and only if you record the right
variable it really works.

Actually we currently don't have any new line insertion fixes for
lines above patch separator.
So, I modified checkpatch a bit to insert (instead of delete) for
co-developed-by.
ie here:

if (WARN("BAD_SIGN_OFF",
         "Co-developed-by: should not be used to attribute nominal patch
author '$author'\n" . "$here\n" . $rawline) &&
    $fix) {
-       fix_delete_line($fixlinenr, $rawline);
+       fix_insert_line($fixlinenr, $rawline);
}


When repeated a few times and removing signed-off-by, this provided an
experience of inserting multiple lines before signed-off-by (as for
each co-developed-by, it is inserting a new co-developed-by)

With this as well, the fix was working as expected, ie, got this after
some iterations of removing signed-off-by and keeping co-developed-by:

...
It is causing boot failures with qemu mac99 in at least some
configurations.occurring
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Co-developed-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/book3s/32/hash.h | 8 ++++----

I did not look into the details. If you are 100% sure that your patch is
right, let us send it to Joe and the mailing list.

Yes, on the basis of observations, it seems to be working correctly.
Sending out the patch :)

I think next a fix for bad sign-off that corrects those obvious typos would
be nice as well.

Sure.

Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees