Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fixes for BAD_SIGN_OFF

On 3/11/20 7:09 pm, Lukas Bulwahn wrote:

On Tue, Nov 3, 2020 at 2:31 PM Aditya Srivastava [off-list ref] wrote:

quoted

Currently, checkpatch warns if the author signs-off two or more times
for the same role.
E.g., for commit 6dd47d9754ff ("mac80211: fix
ieee80211_txq_setup_flows() failure path") we get warning:

WARNING: Duplicate signature
Signed-off-by: Johannes Berg <redacted>

Here there are two Signed-off-by lines present for the same user. So,
we get a warning to remove one.

Similarly, we get warning if the author of the commit signs-off under
co-developed-by.
E.g. for commit 6e88559470f5 ("Documentation: Add section about
CPU vulnerabilities for Spectre") we get:

WARNING: Co-developed-by: should not be used to attribute nominal
patch author 'Tim Chen [off-list ref]'
Co-developed-by: Tim Chen <redacted>

Provide fixes by removing the duplicate signature line and the
co-developed-by line from the commit

The duplicate signature fix is simply wrong, because the indication of
the duplicate signature is imprecise.

Probably, we would first really need to understand the valid cases of
duplicate signature warnings compared to the invalid ones.

Please rework.


Lukas

Oh, Alright. I get it now.
I ran a script over WARNING: Duplicate signature.
I found out that it is occurring with these prefixes and their
frequencies:

Acked-by -- 6
Signed-off-by -- 218
Tested-by -- 3
Reported-by -- 1
Reviewed-by -- 8
Cc -- 28

IMO largely Signed-off-by warnings in this frequency might be false
positives. This is so because we are simply checking for duplicate
occurrences in the sign-offs in checkpatch.pl

Probably, we would first really need to understand the valid cases
of duplicate signature warnings compared to the invalid ones.

Is there any way I can know that the second sign-off is intended?

In my observation, this particular commit was prefixed by '^Link:.*$'
Alternatively, we could use maintainers list to ensure that the signer
is maintainer and avoid this warning.

How about other prefixes. Can we remove them?

I observed one of the 'Tested-by' warnings and it was because of these
sign offs:

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Tested-by:Liang Yang [off-list ref]
Acked-by: Liang Yang <liang.yang@amlogic.com>
Tested-by:Liang Yang [off-list ref]
Acked-by: Liang Yang <liang.yang@amlogic.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Here tested-by is occurring 2 times.

A patch shall only do ONE thing. I see two here.

I'll remove the fix for Duplicate Signature from this patch.


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