[Linux-kernel-mentees] [PATCH v2] checkpatch: add fix option for NETWORKING_BLOCK_COMMENT_STYLE

[Lukas Bulwahn]

On Wed, Nov 25, 2020 at 8:02 AM Aditya <yashsri421 at gmail.com> wrote:
>
> On 22/11/20 12:22 pm, Lukas Bulwahn wrote:
> > On Thu, Nov 19, 2020 at 11:44 AM Aditya <yashsri421 at gmail.com> wrote:
> >>
> >> On 19/11/20 12:30 am, Lukas Bulwahn wrote:
> >>> On Mi., 18. Nov. 2020 at 18:40, Aditya Srivastava <yashsri421 at gmail.com>
> >>> wrote:
> >>>
> >>>> Currently, checkpatch warns us for files in 'net/' and 'drivers/net',
> >>>> if we use an empty '/*' line for comment and contents of comments are
> >>>> in next line
> >>>>
> >>>> E.g., running checkpatch on commit 0d52497ac8ee ("iwlwifi: pcie: remove
> >>>> the refs / unrefs from the transport") reports this warning:
> >>>>
> >>>> WARNING: networking block comments don't use an empty /* line, use /*
> >>>> Comment...
> >>>> +               /*
> >>>> +                * If the TXQ is active, then set the timer, if not,
> >>>>
> >>>> Provide a fix by appending the current line contents to previous line
> >>>> and removing the current line
> >>>>
> >>>
> >>> Patch generally looks good.
> >>>
> >>> Can you check how many comments in net actually follow that style and how
> >>> many follow another style?
> >>>
> >>>
> >>
> >> In drivers/net:
> >> Wrong style: 14695 lines
> >> Correct style: 147961 lines (ie around 10 times)
> >>
> >> In net/:
> >> Wrong style: 5090 lines
> >> Correct style: 30485 lines
> >>
> >
> > Can you find out where the wrong style is used?
> >
> > Maybe the documentation is a bit outdated.
> >
> > For example, some drivers and some subdirectories might have settled
> > for another commenting style.
> >
> > I guess you can submit the fix option, but it could be that the whole
> > rule/feature is broken anyway... so I do not know if that fix is of
> > any good...
> >
> > I think it is better to actually understand, document and encode the
> > current rule that applies.
> >
> > Can you provide an evaluation where the different styles for
> > commenting are aggregated in a good way?
> > E.g., consistently within a file with style XYZ; mixed style but
> > 80-90% are of style ABC; consistent within a directory.
> >
> > If you think some cases are in the wrong style for some specific
> > files, simply send a patch correcting the commenting style and see.
> >
> Hi Lukas
> I have generated the reports regarding the comments used in various
> files at net/ and drivers/net.
> Directory wise reports:
>
> net/:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/networking_block_comments/net/cumulative.txt
>
> drivers/net:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/networking_block_comments/drivers_net/cumulative.txt
>

Can you sort that by the overall number of comments per directory?

> File wise reports:
>
> net/:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/networking_block_comments/net/cumulative_file.txt
>
> drivers/net:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/networking_block_comments/drivers_net/cumulative_file.txt
>
> Some files in 'net/' follow the accepted syntax, such as:
> 'net/batman-adv => 985    0'.
>
> However, some completely not, such as:
> 'net/ax25 => 0    103'.
>
> Some even follow mixed syntax such as:
> 'net/netfilter => 87    125'.
>
> Similarly for drivers/net:
> drivers/net/ethernet/mellanox/mlxsw => 1071    0
> completely follow accepted syntax.
>
> drivers/net/wireless/ralink/rt2x00 => 98, 2082
> follow unaccepted syntax largely.
>
> whereas files such as drivers/net/wireless/ath/ath5k => 304, 486
> follow mixed syntax.
>
> It seems to me as the documentation might be outdated, as you had also
> suggested earlier, or maybe users are unaware of the documentation and
>  just use the syntax for conserving consistency of the code.
>
> What do you think?
>

I think it largely depends on the history of the code and developers
that take care...

We might just start to ask the larger areas to modify the rule for
their directory.

Lukas