Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
On 20/3/21 6:51 pm, Lukas Bulwahn wrote: > On Sat, Mar 20, 2021 at 1:45 PM Aditya Srivastava > wrote: >> >> On 20/3/21 12:23 pm, Aditya wrote: >>> On 18/3/21 11:48 pm, Jonathan Corbet wrote: Lukas Bulwahn writes: > Yeah, and as this line-counting is really just a poor man's > heuristics, we might just be better to really turn this heuristics > into a dedicated cleanup warning script, then we can check for more > indicators, such as "does it contain the word Copyright" somewhere in > the kernel-doc comment, which tells us even more that this is not a > kernel-doc as we would expect it. I really don't think we need that kind of heuristic. The format of kerneldoc comments is fairly rigid; it shouldn't be too hard to pick out the /** comments that don't fit that format, right? Am I missing something there? Thanks, jon >> >> Hi Lukas and Jon! >> I have a question, should I clean up the files with '/**' like >> comments in only header lines? Or as we are planning for making it >> generic, for other lines as well? >> > > Aditya, of course, if you can detect and come across some unintended > '/**' comments in some files, clean them in the same go (as you did > with ecryptfs). > > I am just worried that if you extend it to the fully generic case, > that the list of cases simply explodes: showing many 1,000 cases > across various 1,000 files that need to be cleaned up, and such > clean-up work is just too much to get done by yourself. > > The current list limited to comments in header lines seems to be a set > of patches that you can probably get done. > Sounds good, Lukas. Thanks Aditya
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
On Sat, Mar 20, 2021 at 1:45 PM Aditya Srivastava wrote: > > On 20/3/21 12:23 pm, Aditya wrote: > > On 18/3/21 11:48 pm, Jonathan Corbet wrote: > >> Lukas Bulwahn writes: > >> > >>> Yeah, and as this line-counting is really just a poor man's > >>> heuristics, we might just be better to really turn this heuristics > >>> into a dedicated cleanup warning script, then we can check for more > >>> indicators, such as "does it contain the word Copyright" somewhere in > >>> the kernel-doc comment, which tells us even more that this is not a > >>> kernel-doc as we would expect it. > >> > >> I really don't think we need that kind of heuristic. The format of > >> kerneldoc comments is fairly rigid; it shouldn't be too hard to pick out > >> the /** comments that don't fit that format, right? Am I missing > >> something there? > >> > >> Thanks, > >> > >> jon > >> > > Hi Lukas and Jon! > I have a question, should I clean up the files with '/**' like > comments in only header lines? Or as we are planning for making it > generic, for other lines as well? > Aditya, of course, if you can detect and come across some unintended '/**' comments in some files, clean them in the same go (as you did with ecryptfs). I am just worried that if you extend it to the fully generic case, that the list of cases simply explodes: showing many 1,000 cases across various 1,000 files that need to be cleaned up, and such clean-up work is just too much to get done by yourself. The current list limited to comments in header lines seems to be a set of patches that you can probably get done. Lukas
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
On 20/3/21 12:23 pm, Aditya wrote: > On 18/3/21 11:48 pm, Jonathan Corbet wrote: >> Lukas Bulwahn writes: >> >>> Yeah, and as this line-counting is really just a poor man's >>> heuristics, we might just be better to really turn this heuristics >>> into a dedicated cleanup warning script, then we can check for more >>> indicators, such as "does it contain the word Copyright" somewhere in >>> the kernel-doc comment, which tells us even more that this is not a >>> kernel-doc as we would expect it. >> >> I really don't think we need that kind of heuristic. The format of >> kerneldoc comments is fairly rigid; it shouldn't be too hard to pick out >> the /** comments that don't fit that format, right? Am I missing >> something there? >> >> Thanks, >> >> jon >> Hi Lukas and Jon! I have a question, should I clean up the files with '/**' like comments in only header lines? Or as we are planning for making it generic, for other lines as well? Thanks Aditya
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
On 18/3/21 11:48 pm, Jonathan Corbet wrote: > Lukas Bulwahn writes: > >> Yeah, and as this line-counting is really just a poor man's >> heuristics, we might just be better to really turn this heuristics >> into a dedicated cleanup warning script, then we can check for more >> indicators, such as "does it contain the word Copyright" somewhere in >> the kernel-doc comment, which tells us even more that this is not a >> kernel-doc as we would expect it. > > I really don't think we need that kind of heuristic. The format of > kerneldoc comments is fairly rigid; it shouldn't be too hard to pick out > the /** comments that don't fit that format, right? Am I missing > something there? > > Thanks, > > jon > Thanks for the inputs Lukas and Jonathan. I shall try to come up with something. Thanks Aditya
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
Lukas Bulwahn writes: > Yeah, and as this line-counting is really just a poor man's > heuristics, we might just be better to really turn this heuristics > into a dedicated cleanup warning script, then we can check for more > indicators, such as "does it contain the word Copyright" somewhere in > the kernel-doc comment, which tells us even more that this is not a > kernel-doc as we would expect it. I really don't think we need that kind of heuristic. The format of kerneldoc comments is fairly rigid; it shouldn't be too hard to pick out the /** comments that don't fit that format, right? Am I missing something there? Thanks, jon
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
On Thu, Mar 18, 2021 at 5:37 PM Jonathan Corbet wrote: > > Lukas Bulwahn writes: > > > I wonder if we could extend kernel-doc (not your preferred option as > > it seems) for a new dedicated warning message or maintain a separate > > kernel-doc sanity checking script to emit a dedicated warning based on > > some heuristics that suggests when a "header comment" is probably > > unintentionally declared as a "kernel-doc comment" when it really > > should not be. > > > > Jonathan, would you then prefer to have a separate kernel-doc sanity > > checking script that then allows us to maintain checking for patterns > > we already cleaned up? > > Having a warning in kernel-doc for "This comment starts with /** but > isn't a kerneldoc comment" could be useful, I guess. That is the real > problem, not the fact that it appears at the top of the file. I'm all > for tools that help us to clean things up, but let's not add > line-counting hacks to try to paper it over. > Yeah, and as this line-counting is really just a poor man's heuristics, we might just be better to really turn this heuristics into a dedicated cleanup warning script, then we can check for more indicators, such as "does it contain the word Copyright" somewhere in the kernel-doc comment, which tells us even more that this is not a kernel-doc as we would expect it. Aditya, would you like to try to turn this check into a separate script instead? Lukas
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
Lukas Bulwahn writes: > I wonder if we could extend kernel-doc (not your preferred option as > it seems) for a new dedicated warning message or maintain a separate > kernel-doc sanity checking script to emit a dedicated warning based on > some heuristics that suggests when a "header comment" is probably > unintentionally declared as a "kernel-doc comment" when it really > should not be. > > Jonathan, would you then prefer to have a separate kernel-doc sanity > checking script that then allows us to maintain checking for patterns > we already cleaned up? Having a warning in kernel-doc for "This comment starts with /** but isn't a kerneldoc comment" could be useful, I guess. That is the real problem, not the fact that it appears at the top of the file. I'm all for tools that help us to clean things up, but let's not add line-counting hacks to try to paper it over. Thanks, jon
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
On Mon, Mar 15, 2021 at 8:25 PM Jonathan Corbet wrote: > > Aditya writes: > > >> The opening comment mark /** is used for kernel-doc comments [1] > >> > >> [1] > >> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-format-kernel-doc-comments > >> > > > > Hi Markus! > > That's true. But the content inside the comment does not follow > > kernel-doc format. > > For e.g., try running kernel-doc -none/man/rst on the above file in > > the example("sound/pci/ctxfi/ctresource.c"). > > The starting 2-3 lines in files generally do not contain any > > struct/enum/function, etc. declaration. > > The problem is that it's marked as a kerneldoc comment without actually > being one; it looks like somebody's internal corporate formatting. The > fix is not to put a hack into kernel-doc - we have more than enough of > those in the file already! The right thing to do is to remove the extra > "*" so that the comment doesn't look like a kerneldoc comment anymore. > Jonathan, I agree that that is the right thing to do. Aditya is already following through and is cleaning up the repository. So, let us be optimistic that we will have cleaned up all of those occurrences within a few weeks. But how to continue? Someone is going to come with new files and introduce this pattern again in the repository; and as of now, we do not have a script to identify that pattern and react... Running kernel-doc on the whole tree continuously and just observing the new warnings is probably not going to work as of now: there are 20,000 kernel-doc warnings and at least, I cannot see a really good way to filter out this one specific type of issue among all the warnings that will might appear in the future (without specifically applying Aditya's patch and looking at the diff before and after). I wonder if we could extend kernel-doc (not your preferred option as it seems) for a new dedicated warning message or maintain a separate kernel-doc sanity checking script to emit a dedicated warning based on some heuristics that suggests when a "header comment" is probably unintentionally declared as a "kernel-doc comment" when it really should not be. Jonathan, would you then prefer to have a separate kernel-doc sanity checking script that then allows us to maintain checking for patterns we already cleaned up? Eventually, we might have cleaned up enough to just use kernel-doc and keep it kind of warning-free (as with make htmldocs now) and then, we can simply follow up with kernel-doc and some basic monitoring scripts, but with 20,000 warnings in the whole repository to start with, it is still a long way to that point, IMHO. Lukas
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
Aditya writes: >> The opening comment mark /** is used for kernel-doc comments [1] >> >> [1] >> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-format-kernel-doc-comments >> > > Hi Markus! > That's true. But the content inside the comment does not follow > kernel-doc format. > For e.g., try running kernel-doc -none/man/rst on the above file in > the example("sound/pci/ctxfi/ctresource.c"). > The starting 2-3 lines in files generally do not contain any > struct/enum/function, etc. declaration. The problem is that it's marked as a kerneldoc comment without actually being one; it looks like somebody's internal corporate formatting. The fix is not to put a hack into kernel-doc - we have more than enough of those in the file already! The right thing to do is to remove the extra "*" so that the comment doesn't look like a kerneldoc comment anymore. Thanks, jon
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
On Thu, Mar 11, 2021 at 10:04 PM Aditya wrote: > > On 10/3/21 11:49 am, Lukas Bulwahn wrote: > > On Tue, Mar 9, 2021 at 10:24 PM Aditya wrote: > >> > >> On 9/3/21 7:00 pm, Markus Heiser wrote: > >>> > >>> Am 09.03.21 um 13:53 schrieb Aditya Srivastava: > Starting commented lines in a file mostly contains comments describing > license, copyright or general information about the file. > > E.g., in sound/pci/ctxfi/ctresource.c, initial comment lines describe > its copyright and other related file informations. > >>> > >>> The opening comment mark /** is used for kernel-doc comments [1] > >>> > >>> [1] > >>> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-format-kernel-doc-comments > >>> > >> > >> Hi Markus! > >> That's true. But the content inside the comment does not follow > >> kernel-doc format. > >> For e.g., try running kernel-doc -none/man/rst on the above file in > >> the example("sound/pci/ctxfi/ctresource.c"). > >> The starting 2-3 lines in files generally do not contain any > >> struct/enum/function, etc. declaration. > >> > > > > Aditya, can you provide a diff of the warnings over the whole kernel tree? > > > > At the moment, your patch just implements ignoring the initial > > comment, which probably is good for experimentation. > > > > Alternatively, we could simply have a dedicated warning and then > > ignore it or even warn and then parse it as-if. > > > > In the "long run", we would probably want to fix all current files in > > the repository by just replacing '/**' by '/*' and have kernel-doc > > warn about this suspicious pattern, when new files appear (maybe even > > configurable, but that is another feature to enable or disable certain > > kernel-doc checks and warnings). I would certainly assist and > > contribute to such a clean-up task. > > > > I think the first step is to look at the diff, and see how many cases > > really appear in the tree... then check how many patches throughout > > the whole tree are required and if they are generally accepted. > > > > Hi Lukas! > This is the diff of the warnings over kernel tree before and after > applying these changes. > There are 2 sections in this report: > 1) for the warnings present before, but not after, and; > 2) after but not before > > The part (2) contains, for some cases, where the warning for "warning: > Incorrect use of kernel-doc format:" type has changed to "warning: > wrong kernel-doc identifier on line:" type. > > The diff file can be found at: > https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/kernel-doc/avoid_init_line_diff.txt > Thanks, let us check if we can use this diff to create a patch set that cleans up those header comments for those files. Lukas
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
On 10/3/21 11:49 am, Lukas Bulwahn wrote: > On Tue, Mar 9, 2021 at 10:24 PM Aditya wrote: >> >> On 9/3/21 7:00 pm, Markus Heiser wrote: >>> >>> Am 09.03.21 um 13:53 schrieb Aditya Srivastava: Starting commented lines in a file mostly contains comments describing license, copyright or general information about the file. E.g., in sound/pci/ctxfi/ctresource.c, initial comment lines describe its copyright and other related file informations. >>> >>> The opening comment mark /** is used for kernel-doc comments [1] >>> >>> [1] >>> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-format-kernel-doc-comments >>> >> >> Hi Markus! >> That's true. But the content inside the comment does not follow >> kernel-doc format. >> For e.g., try running kernel-doc -none/man/rst on the above file in >> the example("sound/pci/ctxfi/ctresource.c"). >> The starting 2-3 lines in files generally do not contain any >> struct/enum/function, etc. declaration. >> > > Aditya, can you provide a diff of the warnings over the whole kernel tree? > > At the moment, your patch just implements ignoring the initial > comment, which probably is good for experimentation. > > Alternatively, we could simply have a dedicated warning and then > ignore it or even warn and then parse it as-if. > > In the "long run", we would probably want to fix all current files in > the repository by just replacing '/**' by '/*' and have kernel-doc > warn about this suspicious pattern, when new files appear (maybe even > configurable, but that is another feature to enable or disable certain > kernel-doc checks and warnings). I would certainly assist and > contribute to such a clean-up task. > > I think the first step is to look at the diff, and see how many cases > really appear in the tree... then check how many patches throughout > the whole tree are required and if they are generally accepted. > Hi Lukas! This is the diff of the warnings over kernel tree before and after applying these changes. There are 2 sections in this report: 1) for the warnings present before, but not after, and; 2) after but not before The part (2) contains, for some cases, where the warning for "warning: Incorrect use of kernel-doc format:" type has changed to "warning: wrong kernel-doc identifier on line:" type. The diff file can be found at: https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/kernel-doc/avoid_init_line_diff.txt Thanks Aditya
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
On Tue, Mar 9, 2021 at 10:24 PM Aditya wrote: > > On 9/3/21 7:00 pm, Markus Heiser wrote: > > > > Am 09.03.21 um 13:53 schrieb Aditya Srivastava: > >> Starting commented lines in a file mostly contains comments describing > >> license, copyright or general information about the file. > >> > >> E.g., in sound/pci/ctxfi/ctresource.c, initial comment lines describe > >> its copyright and other related file informations. > > > > The opening comment mark /** is used for kernel-doc comments [1] > > > > [1] > > https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-format-kernel-doc-comments > > > > Hi Markus! > That's true. But the content inside the comment does not follow > kernel-doc format. > For e.g., try running kernel-doc -none/man/rst on the above file in > the example("sound/pci/ctxfi/ctresource.c"). > The starting 2-3 lines in files generally do not contain any > struct/enum/function, etc. declaration. > Aditya, can you provide a diff of the warnings over the whole kernel tree? At the moment, your patch just implements ignoring the initial comment, which probably is good for experimentation. Alternatively, we could simply have a dedicated warning and then ignore it or even warn and then parse it as-if. In the "long run", we would probably want to fix all current files in the repository by just replacing '/**' by '/*' and have kernel-doc warn about this suspicious pattern, when new files appear (maybe even configurable, but that is another feature to enable or disable certain kernel-doc checks and warnings). I would certainly assist and contribute to such a clean-up task. I think the first step is to look at the diff, and see how many cases really appear in the tree... then check how many patches throughout the whole tree are required and if they are generally accepted. Lukas
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
On 9/3/21 7:00 pm, Markus Heiser wrote: > > Am 09.03.21 um 13:53 schrieb Aditya Srivastava: >> Starting commented lines in a file mostly contains comments describing >> license, copyright or general information about the file. >> >> E.g., in sound/pci/ctxfi/ctresource.c, initial comment lines describe >> its copyright and other related file informations. > > The opening comment mark /** is used for kernel-doc comments [1] > > [1] > https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-format-kernel-doc-comments > Hi Markus! That's true. But the content inside the comment does not follow kernel-doc format. For e.g., try running kernel-doc -none/man/rst on the above file in the example("sound/pci/ctxfi/ctresource.c"). The starting 2-3 lines in files generally do not contain any struct/enum/function, etc. declaration. Thanks Aditya
Re: [RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
Am 09.03.21 um 13:53 schrieb Aditya Srivastava: Starting commented lines in a file mostly contains comments describing license, copyright or general information about the file. E.g., in sound/pci/ctxfi/ctresource.c, initial comment lines describe its copyright and other related file informations. The opening comment mark /** is used for kernel-doc comments [1] [1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-format-kernel-doc-comments -- Markus -- But as kernel-doc reads these lines, it results in ineffective warnings by kernel-doc, related to these. Provide a simple fix by skipping first three lines in a file for checking kernel-doc comments. Suggested-by: Lukas Bulwahn Signed-off-by: Aditya Srivastava --- scripts/kernel-doc | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/kernel-doc b/scripts/kernel-doc index e1e562b2e2e7..431add05248e 100755 --- a/scripts/kernel-doc +++ b/scripts/kernel-doc @@ -2375,6 +2375,7 @@ sub process_file($) { my $file; my $initial_section_counter = $section_counter; my ($orig_file) = @_; +my $lineno = 0;# to maintain the count of line number in a file $file = map_filename($orig_file); @@ -2388,13 +2389,16 @@ sub process_file($) { $section_counter = 0; while () { + $lineno++; while (s/\\\s*$//) { $_ .= ; + $lineno++; } # Replace tabs by spaces while ($_ =~ s/\t+/' ' x (length($&) * 8 - length($`) % 8)/e) {}; # Hand this line to the appropriate state handler - if ($state == STATE_NORMAL) { + if ($state == STATE_NORMAL + && $lineno > 3) {# to avoid starting comment lines describing the file process_normal(); } elsif ($state == STATE_NAME) { process_name($file, $_);
[RFC] scripts: kernel-doc: avoid warnings due to initial commented lines in file
Starting commented lines in a file mostly contains comments describing license, copyright or general information about the file. E.g., in sound/pci/ctxfi/ctresource.c, initial comment lines describe its copyright and other related file informations. But as kernel-doc reads these lines, it results in ineffective warnings by kernel-doc, related to these. Provide a simple fix by skipping first three lines in a file for checking kernel-doc comments. Suggested-by: Lukas Bulwahn Signed-off-by: Aditya Srivastava --- scripts/kernel-doc | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/kernel-doc b/scripts/kernel-doc index e1e562b2e2e7..431add05248e 100755 --- a/scripts/kernel-doc +++ b/scripts/kernel-doc @@ -2375,6 +2375,7 @@ sub process_file($) { my $file; my $initial_section_counter = $section_counter; my ($orig_file) = @_; +my $lineno = 0;# to maintain the count of line number in a file $file = map_filename($orig_file); @@ -2388,13 +2389,16 @@ sub process_file($) { $section_counter = 0; while () { + $lineno++; while (s/\\\s*$//) { $_ .= ; + $lineno++; } # Replace tabs by spaces while ($_ =~ s/\t+/' ' x (length($&) * 8 - length($`) % 8)/e) {}; # Hand this line to the appropriate state handler - if ($state == STATE_NORMAL) { + if ($state == STATE_NORMAL + && $lineno > 3) { # to avoid starting comment lines describing the file process_normal(); } elsif ($state == STATE_NAME) { process_name($file, $_); -- 2.17.1