On Thu, 13 Aug 2020, ishan srivastava wrote:

> Hello Lukas, thanks for the reply. I have been trying to come up with a solution to the bug https://bugzilla.kernel.org/show_bug.cgi?id=207085 and I have to say that I haven’t been able to come up with one that is neat and clean.
>

Okay, let us start easy here.

1. Please always CC: linux-kernel-mentees@lists.linuxfoundation.org.
2. Your email client is broken; you will not be able to communicate with 
the kernel community with a broken email client. Please follow the 
recommendation:
https://www.kernel.org/doc/html/latest/process/email-clients.html#email-clients. 
 
> The problem described in the bug refers to false positive warnings that are encountered for missing blank lines after declarations inside a struct.
> While I found no rule regarding this on https://www.kernel.org/doc/html/v4.10/process/coding-style.html#codingstyle, I presumed that this is a problem and went ahead to devise a solution.
>

3. What is the latest kernel version? From which year is the documentation 
you looked at and referred to? Please always look at the latest version 
(at least the latest tag in torvalds' tree) of the documentation and 
refer to that.

> The code in checkpatch.pl lines 3484-3523 ensures that there is a blank line in between declarations. To ensure that these checks are skipped when the line being checked by checkpatch.pl is inside a struct, we need to maintain a
> counter as to the depth of nested structs the current line in and skip the checks whenever inside even a single struct (keeping aside further complications like do different struct declarations still need line break between them)
>

4. Why are line numbers alone meaningless in an email? Think about the 
needed information that the reader can actually know what part of code you 
are referring to? (Hint: There is only one version of the kernel sources 
and it has not changed a single bit in the last 15 years, right?)
What conveys the complete state of a git source code repository in a short 
and unambiguous way?

Now to the technical discussion:

> If I have conveyed my concerns correctly then we can see that we will introduce a level of complication to checkpatch.pl as to which kind of paradigms the current line is in, which is currently not supported.
> 
> I can go ahead and submit a patch and also show that to you if you wish so but I am not sure whether it would be accepted by the maintainers as from what I have inferred, this level of complication hasn’t been introduced in
> checkpatch.pl.
>

Has this issue of a false positive from checkpatch.pl appeared on 
multiple commits in the last 10 years? How about finding out how often 
this rule would have emitted a true positive and a false positive on the 
commits of the last 10 years?

> I believe some other linters or static analysis tools can perform this with a few lines of code in their config file.
>

That is certainly an alternative; do you a specific tool in mind and can 
you share the configuration to obtain the cases you consider true positive 
findings?
 
> Please let me know what you think about this and what should be my next steps regarding this. I also find this interesting since even though not mentioned anywhere I see this coding style being followed in most places and might just be
> one of the false positives that we are looking forward to fix through this project. I would also like to know if you have any framework/methodology in mind to approaching these problem (as this is a problem that needs to be addressed
> by this project) or should we directly introduce this complexity to checkpatch.pl?
>

Please address the points 1. to 4. correctly first; if you cannot get that 
solved, set up and you cannot follow the process, there is no chance for a 
kernel-related mentorship.

Once you solved that, think about how to send an email correctly now and 
compare it to first email. Send this improved email addressing the points 
1 to 4. above. Then we can continue the technical discussion.

Lukas