From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D506DC43461 for ; Mon, 14 Sep 2020 12:31:58 +0000 (UTC) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5F76220709 for ; Mon, 14 Sep 2020 12:31:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="E23WLdZl" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5F76220709 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linux-kernel-mentees-bounces@lists.linuxfoundation.org Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 1ABBD8706D; Mon, 14 Sep 2020 12:31:58 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8tzMdtdSnQny; Mon, 14 Sep 2020 12:31:57 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 2CEEF87044; Mon, 14 Sep 2020 12:31:57 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 15892C0859; Mon, 14 Sep 2020 12:31:57 +0000 (UTC) Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 851FCC0051 for ; Mon, 14 Sep 2020 12:31:55 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 7997D85FA3 for ; Mon, 14 Sep 2020 12:31:55 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hUoA6QTomJAp for ; Mon, 14 Sep 2020 12:31:54 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mail-oi1-f193.google.com (mail-oi1-f193.google.com [209.85.167.193]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 8BF5885F95 for ; Mon, 14 Sep 2020 12:31:54 +0000 (UTC) Received: by mail-oi1-f193.google.com with SMTP id t76so17646776oif.7 for ; Mon, 14 Sep 2020 05:31:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=M8q0gPqVC4EFxPMTPdnlD0uYN4pEPXykZI0OdOJRRk4=; b=E23WLdZlCG4h1PuT9AOedlWPKnRkhk1ea7ZNRb7VOJ52+syWCJ6Lwa4lzh1AYSHIzT 94ZsdclVVbl8vCE4qPLTYA0QElCsMsv75MlY5AI/5z1PKRM8vkNjhfNKJjSoRHvvy/lx 6F8PEddWSmgC70qt9euSyDpytK9L7qqZFWDgh0vQOhp1ZtXva3r9fEddcfFTzbiSp4Lb YrDa8+IuUF9pLXcv+0cC2s3o8lY4I6MLk76EGreKGy8/Tzd2ZpTQ6rK/I7lZmCTsPKF2 kmtPkMoC3R/687rKl/ud7Uv6SfKP1wAYXtCPDpIrnjWnY6uhEPpkuZzjAZVRsoSO3FQA CXtA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=M8q0gPqVC4EFxPMTPdnlD0uYN4pEPXykZI0OdOJRRk4=; b=GBrb8VheZyGJTYBd9aGSHVlWk6+MPmwpClqmyE9xNEdppXY7lJyWtE/zeAylduhwaJ KuB861wpYrUoB9vvhYy1glMAT0uR1SHMFt8aJLe2l6olvO6Xz+2DVnSZjeq/PTgDfmKG zpZ9aYqMLv9YfpQ90qGm0cuNk/ml/D0N7pSOe87dd2fw5esLiJJFvIiXZ9pEd0iyfWyv WzcX9Sf/UUJkOOx+tVLK4eeVwfpIjW7i4cvdRgZDpC0NPXlIPk6qH+CHd0VkcGe1XYRk oGE6dwPJMZIm2MCUxhRldlqpZySb2hXSEt7YS6uANJWTsPbctWGqebzrFfkdIBZcTiEN 24dw== X-Gm-Message-State: AOAM532kENrr9lYSxIs7xrSXC2vDMwsbvNm5y7F4+1PLGhuxHcVXG2gf CeGlM/vg8N3BcWxRuXA/AYLZYMEvX/QNQNTX4sI= X-Google-Smtp-Source: ABdhPJxfwH1xhqFTJ9sKhzGh7BIie+Gx4a/JT1emzvzwhy3JkInAZx65EGsi/S1PYRzi1iFlCiXecZmybE9t7Ge9eN4= X-Received: by 2002:aca:4d58:: with SMTP id a85mr8913868oib.110.1600086713497; Mon, 14 Sep 2020 05:31:53 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Dwaipayan Ray Date: Mon, 14 Sep 2020 18:01:37 +0530 Message-ID: To: Lukas Bulwahn Cc: linux-kernel-mentees@lists.linuxfoundation.org Subject: Re: [Linux-kernel-mentees] Linux kernel checkpatch.pl mentorship X-BeenThere: linux-kernel-mentees@lists.linuxfoundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: multipart/mixed; boundary="===============1490591641971228485==" Errors-To: linux-kernel-mentees-bounces@lists.linuxfoundation.org Sender: "Linux-kernel-mentees" --===============1490591641971228485== Content-Type: multipart/alternative; boundary="0000000000002afc9405af45389f" --0000000000002afc9405af45389f Content-Type: text/plain; charset="UTF-8" On Mon, Sep 14, 2020 at 10:47 AM Lukas Bulwahn wrote: > > > > On Mon, 14 Sep 2020, Dwaipayan Ray wrote: > > > > > > > > You should also try to find a case where checkpatch.pl can be improved: > > > > > > - It is best if you look at the checkpatch.pl error/warning reports and > > > check if you can find a class of typical false positives in the reported > > > data. Then, think about how to make the pattern in checkpatch and propose > > > a patch to checkpatch.pl on this mailing list; with the evaluation data > > > backing that your patch really improves the situation. We will then test > > > and check your patch as well. > > > > > > - If you really cannot find a typical false positive, maybe you can take > > > the task from Ayush to fix checkpatch with git ranges? > > > > > Hi,I will try to find and evaluate the possibiltiy of such a case, and report my finding > > as soon as I find something substantial. > > > > I may also require some advice from you on the patch submission process. > > > > That is what a mentor is there for, but try hard to understand the > warnings yourself first. This is a mentorship, not a tutorial. > > Lukas Hi, I looked into the checkpatch.pl 's output I collected. And I have found some possible candidates but I would like your opinion on them. The first one is double warnings for the same kind of coding style error. If an SPDX tag is in the following format: "// SPDX-License-Identifier: GPL-2.0", then checkpatch.pl generates two warnings: 1) Improper SPDX comment style for ... , please use '/*' instead. 2) Missing or malformed SPDX-License-Identifier tag in line 1. Example: commit 726721a51838 Warnings generated: WARNING:SPDX_LICENSE_TAG: Improper SPDX comment style for 'kernel/trace/trace_synth.h', please use '/*' instead #4041: FILE: kernel/trace/trace_synth.h:1: +// SPDX-License-Identifier: GPL-2.0 WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1 #4041: FILE: kernel/trace/trace_synth.h:1: +// SPDX-License-Identifier: GPL-2.0 In my opinion, only one warning should suffice if a '//' comment style is used in spdx tag. But I would like to know your view on it. ---------------------------------------------------------------------------------------------- The second candidate is related to the following warning: WARNING:SPDX_LICENSE_TAG: DT binding documents should be licensed (GPL-2.0-only OR BSD-2-Clause) #110: FILE: Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml:1: +# SPDX-License-Identifier: GPL-2.0-only In checkpath.pl line 3209: $fixed[$fixlinenr] =~ s/SPDX-License-Identifier: .*/SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)/; The desired fix is "(GPL-2.0-only OR BSD-2-Clause)" . Shouldn't the identifier be "GPL-2.0-only" or "BSD-2-Clause", that is either of them? Or was the former the decided one? ---------------------------------------------------------------------------------------------- The third candidate is related to the warning: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author ... I found several such commits in which the author had used different mail addresses in the signed-off -by section, due to which this warning is generated. An example is Commit dc5bdb68b5b3 . Git log show: Author: Daniel Vetter ....,. Signed-off-by: Daniel Vetter This is infact a common scenario. I easily found another commit 207324a321a8. Git log shows: Author: Minas Harutyunyan ... Signed-off-by: Minas Harutyunyan This warning could be avoided or at least handled better. I would like to know if any of them can be worked on. Thanks, Dwaipayan. --0000000000002afc9405af45389f Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable



On Mon, Sep 14, 2020 a= t 10:47 AM Lukas Bulwahn <luk= as.bulwahn@gmail.com> wrote:
>
>
>
> On Mon,= 14 Sep 2020, Dwaipayan Ray wrote:
>
> >
> >
>= ; > > You should also try to find a case where checkpatch.pl can be improved:
> > >
> >= ; > - It is best if you look at the che= ckpatch.pl error/warning reports and
> > > check if you can= find a class of typical false positives in the reported
> > > = data. Then, think about how to make the pattern in checkpatch and propose> > > a patch to checkpatch.pl<= /a> on this mailing list; with the evaluation data
> > > backin= g that your patch really improves the situation. We will then test
> = > > and check your patch as well.
> > >
> > >= - If you really cannot find a typical false positive, maybe you can take> > > the task from Ayush to fix checkpatch with git ranges?
= > > >
> > Hi,I will try to find and evaluate the possibil= tiy of such a case, and report my finding
> > as soon as I find s= omething substantial.
> >
> > I may also require some adv= ice from you on the patch submission process.
> >
>
> = That is what a mentor is there for, but try hard to understand the
> = warnings yourself first. This is a mentorship, not a tutorial.
>
&= gt; Lukas

I looked into = the checkpatch.pl 's output I coll= ected. And I have found some possible candidates
but I would like your opinion on them.

The first= one is double warnings for the same kind of coding style error.
If an SPDX tag is in the following format: &= quot;// SPDX-License-Identifier: GPL-2.0",
then checkpatch.pl generates two warnings:
=

1) Improper SPDX comment style for ... , please use = 9;/*' instead.
2)=C2=A0Missing or malformed SPDX-License-Iden= tifier tag in line 1.

Example: commit=C2=A0726721a= 51838
Warnings generated:

WARNING:SP= DX_LICENSE_TAG: Improper SPDX comment style for 'kernel/trace/trace_syn= th.h', please use '/*' instead
#4041: FILE: kernel/trace/tra= ce_synth.h:1:
+// SPDX-License-Identifier: GPL-2.0

WARNING:SPDX_L= ICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#= 4041: FILE: kernel/trace/trace_synth.h:1:
+// SPDX-License-Identifier: G= PL-2.0

In my opinion, only one warning should suff= ice if a '//' comment style is used in spdx tag.=C2=A0
Bu= t I would like to know your view on it.

----------= ---------------------------------------------------------------------------= ---------
The second candidate is related to the following warnin= g:

WARNING:SPDX_LICENSE_TAG: DT binding documents = should be licensed (GPL-2.0-only OR BSD-2-Clause)
#110: FILE: Documentat= ion/devicetree/bindings/i2c/snps,designware-i2c.yaml:1:
+# SPDX-License-= Identifier: GPL-2.0-only

In checkpath.pl line 3209:
=C2=A0$fixed[$fixlinenr= ] =3D~ s/SPDX-License-Identifier: .*/SPDX-License-Identifier: (GPL-2.0-only= OR BSD-2-Clause)/;

The desired fix is "(= GPL-2.0-only OR BSD-2-Clause)" . Shouldn't the identifier be "= ;GPL-2.0-only"=C2=A0
or "BSD-2-Clause", that is ei= ther of them? Or was the former the decided one?

-= ---------------------------------------------------------------------------= ------------------
The third candidate is related to the warning:=

WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by= : line by nominal patch author ...

I found sev= eral such commits in which the author had used different mail addresses in = the=C2=A0
signed-off -by section, due to which this warning is ge= nerated.

An example is Commit=C2=A0dc5bdb68b5b3 .<= /div>
Git log show:
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
....,.<= /div>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Thi= s is infact a common scenario. I easily found another commit 207324a321a8.<= /div>
Git log shows:
Author: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com&= gt;
...
Signed-off-by: Minas Harutyunyan <hminas@synopsys.com>
=
This warning could be avoided or at least handled better.


I would like to know if any of them c= an be worked on.

Thanks,
Dwaipayan.


--0000000000002afc9405af45389f-- --===============1490591641971228485== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees --===============1490591641971228485==--