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

Aditya yashsri421 at gmail.com
Wed Nov 25 07:02:44 UTC 2020


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

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?

Thanks
Aditya

> Lukas
>


More information about the Linux-kernel-mentees mailing list