<div><br></div><div><br><div class="gmail_quote"><div dir="auto">On Di., 10. Nov. 2020 at 19:13, Aditya <<a href="mailto:yashsri421@gmail.com">yashsri421@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 10/11/20 8:52 pm, Lukas Bulwahn wrote:<br>
> On Tue, Nov 10, 2020 at 4:13 PM Aditya <<a href="mailto:yashsri421@gmail.com" target="_blank">yashsri421@gmail.com</a>> wrote:<br>
>><br>
>> On 10/11/20 7:30 pm, Lukas Bulwahn wrote:<br>
>>> On Tue, Nov 10, 2020 at 2:06 PM Aditya Srivastava <<a href="mailto:yashsri421@gmail.com" target="_blank">yashsri421@gmail.com</a>> wrote:<br>
>>>><br>
>>>> Currently checkpatch warns us if there is no 'Signed-off-by' line<br>
>>>> for the patch.<br>
>>>><br>
>>>> E.g., running checkpatch on commit 9ac060a708e0 ("leaking_addresses:<br>
>>>> Completely remove --version flag") reports this error:<br>
>>>><br>
>>>> ERROR: Missing Signed-off-by: line(s)<br>
>>>><br>
>>>> Provide a fix by adding a Signed-off-by line corresponding to the author<br>
>>>> of the patch before the patch separator line. Also avoid this error for<br>
>>>> the commits where some typo is present in the sign off.<br>
>>>><br>
>>>> E.g. for commit 8cde5d5f7361 ("bus: ti-sysc: Detect omap4 type timers<br>
>>>> for quirk") we get missing sign off as well as bad sign off for:<br>
>>>><br>
>>>> Siganed-off-by: Tony Lindgren <<a href="mailto:tony@atomide.com" target="_blank">tony@atomide.com</a>><br>
>>>><br>
>>>> Here it is probably best to give BAD_SIGN_OFF warning for Non-standard<br>
>>>> signature and avoid MISSING_SIGN_OFF<br>
>>>><br>
>>>> Suggested-by: Joe Perches <<a href="mailto:joe@perches.com" target="_blank">joe@perches.com</a>><br>
>>>> Signed-off-by: Aditya Srivastava <<a href="mailto:yashsri421@gmail.com" target="_blank">yashsri421@gmail.com</a>><br>
>>>> ---<br>
>>>> This patch was made after applying <a href="https://lore.kernel.org/linux-kernel-mentees/20201108100632.75340-1-dwaipayanray1@gmail.com/" rel="noreferrer" target="_blank">https://lore.kernel.org/linux-kernel-mentees/20201108100632.75340-1-dwaipayanray1@gmail.com/</a><br>
>>>> and my last changes at <a href="https://lore.kernel.org/linux-kernel-mentees/20201108134317.25400-1-yashsri421@gmail.com/" rel="noreferrer" target="_blank">https://lore.kernel.org/linux-kernel-mentees/20201108134317.25400-1-yashsri421@gmail.com/</a><br>
>>>><br>
>>>>  scripts/<a href="http://checkpatch.pl" rel="noreferrer" target="_blank">checkpatch.pl</a> | 18 +++++++++++++++---<br>
>>>>  1 file changed, 15 insertions(+), 3 deletions(-)<br>
>>>><br>
>>>> diff --git a/scripts/<a href="http://checkpatch.pl" rel="noreferrer" target="_blank">checkpatch.pl</a> b/scripts/<a href="http://checkpatch.pl" rel="noreferrer" target="_blank">checkpatch.pl</a><br>
>>>> index cb46288127ac..2deffd0c091b 100755<br>
>>>> --- a/scripts/<a href="http://checkpatch.pl" rel="noreferrer" target="_blank">checkpatch.pl</a><br>
>>>> +++ b/scripts/<a href="http://checkpatch.pl" rel="noreferrer" target="_blank">checkpatch.pl</a><br>
>>>> @@ -2404,6 +2404,8 @@ sub process {<br>
>>>><br>
>>>>         my $last_blank_line = 0;<br>
>>>>         my $last_coalesced_string_linenr = -1;<br>
>>>> +       my $patch_separator_linenr = 0;<br>
>>>> +       my $non_standard_signature = 0;<br>
>>>><br>
>>>>         our @report = ();<br>
>>>>         our $cnt_lines = 0;<br>
>>>> @@ -2755,6 +2757,10 @@ sub process {<br>
>>>>                 if ($line =~ /^---$/) {<br>
>>>>                         $has_patch_separator = 1;<br>
>>>>                         $in_commit_log = 0;<br>
>>>> +                       # to add missing sign off line before diff(s)<br>
>>>> +                       if($patch_separator_linenr == 0) {<br>
>>>> +                               $patch_separator_linenr = $linenr;<br>
>>><br>
>>> Well are you sure linenr is correct? (As I wrote, I do not know, but<br>
>>> you should know.)<br>
>>><br>
>>> I think you would need to create a patch for testing your feature<br>
>>> where multiple things are fixed within the same patch and then check<br>
>>> if the fix is added at the right place.<br>
>>><br>
>><br>
>> I checked the fix over following patch.<br>
>><br>
>> I have modified the patch such that it has co-developed-by as the<br>
>> author(nominal patch author warning, which requires this line to be<br>
>> deleted)<br>
>><br>
>> Also 'occuring', which is a typo(where the typo needs to be replaced)<br>
>><br>
>> It works as expected.<br>
>><br>
> <br>
> Well to really check if it works as expected, you need to have a test<br>
> patch where a fix adds multiple lines into the fixed patch before the<br>
> line you try to record.<br>
> <br>
> Then, fixlinenr and linenr are probably more than just 1 line apart,<br>
> but really multiple lines apart and only if you record the right<br>
> variable it really works.<br>
> <br>
<br>
Actually we currently don't have any new line insertion fixes for<br>
lines above patch separator.<br>
So, I modified checkpatch a bit to insert (instead of delete) for<br>
co-developed-by.<br>
ie here:<br>
<br>
if (WARN("BAD_SIGN_OFF",<br>
         "Co-developed-by: should not be used to attribute nominal patch<br>
author '$author'\n" . "$here\n" . $rawline) &&<br>
    $fix) {<br>
-       fix_delete_line($fixlinenr, $rawline);<br>
+       fix_insert_line($fixlinenr, $rawline);<br>
}<br>
<br>
<br>
When repeated a few times and removing signed-off-by, this provided an<br>
experience of inserting multiple lines before signed-off-by (as for<br>
each co-developed-by, it is inserting a new co-developed-by)<br>
<br>
With this as well, the fix was working as expected, ie, got this after<br>
some iterations of removing signed-off-by and keeping co-developed-by:<br>
<br>
...<br>
It is causing boot failures with qemu mac99 in at least some<br>
configurations.occurring<br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Co-developed-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
Signed-off-by: Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" target="_blank">mpe@ellerman.id.au</a>><br>
---<br>
 arch/powerpc/include/asm/book3s/32/hash.h | 8 ++++----<br>
<br>
</blockquote><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">I think next a fix for bad sign-off that corrects those obvious typos would be nice as well.</div><div dir="auto"><br></div><div dir="auto">Lukas</div><div dir="auto"><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
Aditya<br>
<br>
> Other than that, it is all good.<br>
> <br>
> Lukas<br>
> <br>
<br>
</blockquote></div></div>