[PATCH] scripts/kernel-doc: Get -export option working again

[PATCH] scripts/kernel-doc: Get -export option working again

[Akira Yokosawa]

Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
literal"), exported symbols marked by EXPORT_SYMBOL_NS(_GPL) are
ignored by "kernel-doc -export" in fresh build of "make htmldocs".

This is because regex in the perl script for those markers fails to
match the new signatures:

  - EXPORT_SYMBOL_NS(symbol, "ns");
  - EXPORT_SYMBOL_NS_GPL(symbol, "ns");

Update the regex so that it matches quoted string.

Note: Escape sequence of \w is good for C identifiers, but can be
too strict for quoted strings.  Instead, use \S, which matches any
non-whitespace character, for compatibility with possible extension
of namespace convention in the future [1].

Fixes: cdd30ebb1b9f ("module: Convert symbol namespace to string literal")
Link: https://lore.kernel.org/CAK7LNATMufXP0EA6QUE9hBkZMa6vJO6ZiaYuak2AhOrd2nSVKQ@mail.gmail.com/ [1]
Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
Cc: Masahiro Yamada <masahiroy@kernel.org>
---
This fixes regression in v6.13-rc2.

Quick reproducer:

    ./script/kernel-doc -rst -export drivers/iommu/iommufd/device.c

On v6.13-rc2, kernel-doc will say:

     drivers/iommu/iommufd/device.c:1: warning: no structured comments found

With this patch applied, you'll get reST formatted kernel-doc comments. 

Akira
 
---
 scripts/kernel-doc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index f66070176ba3..4ee843d3600e 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -267,7 +267,7 @@ my $doc_inline_sect = '\s*\*\s*(@\s*[\w][\w\.]*\s*):(.*)';
 my $doc_inline_end = '^\s*\*/\s*$';
 my $doc_inline_oneline = '^\s*/\*\*\s*(@[\w\s]+):\s*(.*)\s*\*/\s*$';
 my $export_symbol = '^\s*EXPORT_SYMBOL(_GPL)?\s*\(\s*(\w+)\s*\)\s*;';
-my $export_symbol_ns = '^\s*EXPORT_SYMBOL_NS(_GPL)?\s*\(\s*(\w+)\s*,\s*\w+\)\s*;';
+my $export_symbol_ns = '^\s*EXPORT_SYMBOL_NS(_GPL)?\s*\(\s*(\w+)\s*,\s*"\S+"\)\s*;';
 my $function_pointer = qr{([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)};
 my $attribute = qr{__attribute__\s*\(\([a-z0-9,_\*\s\(\)]*\)\)}i;
 

base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
-- 
2.34.1

Re: [PATCH] scripts/kernel-doc: Get -export option working again

[Randy Dunlap]

Hi Akira,


On 12/10/24 3:04 AM, Akira Yokosawa wrote:
> Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
> literal"), exported symbols marked by EXPORT_SYMBOL_NS(_GPL) are
> ignored by "kernel-doc -export" in fresh build of "make htmldocs".
> 
> This is because regex in the perl script for those markers fails to
> match the new signatures:
> 
>   - EXPORT_SYMBOL_NS(symbol, "ns");
>   - EXPORT_SYMBOL_NS_GPL(symbol, "ns");
> 
> Update the regex so that it matches quoted string.
> 
> Note: Escape sequence of \w is good for C identifiers, but can be
> too strict for quoted strings.  Instead, use \S, which matches any
> non-whitespace character, for compatibility with possible extension
> of namespace convention in the future [1].
> 
> Fixes: cdd30ebb1b9f ("module: Convert symbol namespace to string literal")
> Link: https://lore.kernel.org/CAK7LNATMufXP0EA6QUE9hBkZMa6vJO6ZiaYuak2AhOrd2nSVKQ@mail.gmail.com/ [1]
> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> ---
> This fixes regression in v6.13-rc2.
> 
> Quick reproducer:
> 
>     ./script/kernel-doc -rst -export drivers/iommu/iommufd/device.c
> 
> On v6.13-rc2, kernel-doc will say:
> 
>      drivers/iommu/iommufd/device.c:1: warning: no structured comments found
> 

I wondered where those warnings were coming from.
Thanks for the explanation and fix.

Tested-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Randy Dunlap <rdunlap@infradead.org>


> With this patch applied, you'll get reST formatted kernel-doc comments. 
> 
> Akira
>  
> ---
>  scripts/kernel-doc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index f66070176ba3..4ee843d3600e 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -267,7 +267,7 @@ my $doc_inline_sect = '\s*\*\s*(@\s*[\w][\w\.]*\s*):(.*)';
>  my $doc_inline_end = '^\s*\*/\s*$';
>  my $doc_inline_oneline = '^\s*/\*\*\s*(@[\w\s]+):\s*(.*)\s*\*/\s*$';
>  my $export_symbol = '^\s*EXPORT_SYMBOL(_GPL)?\s*\(\s*(\w+)\s*\)\s*;';
> -my $export_symbol_ns = '^\s*EXPORT_SYMBOL_NS(_GPL)?\s*\(\s*(\w+)\s*,\s*\w+\)\s*;';
> +my $export_symbol_ns = '^\s*EXPORT_SYMBOL_NS(_GPL)?\s*\(\s*(\w+)\s*,\s*"\S+"\)\s*;';
>  my $function_pointer = qr{([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)};
>  my $attribute = qr{__attribute__\s*\(\([a-z0-9,_\*\s\(\)]*\)\)}i;
>  
> 
> base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4

-- 
~Randy

Re: [PATCH] scripts/kernel-doc: Get -export option working again

[Matthew Wilcox]

On Tue, Dec 10, 2024 at 08:04:15PM +0900, Akira Yokosawa wrote:
> +++ b/scripts/kernel-doc
> @@ -267,7 +267,7 @@ my $doc_inline_sect = '\s*\*\s*(@\s*[\w][\w\.]*\s*):(.*)';
>  my $doc_inline_end = '^\s*\*/\s*$';
>  my $doc_inline_oneline = '^\s*/\*\*\s*(@[\w\s]+):\s*(.*)\s*\*/\s*$';
>  my $export_symbol = '^\s*EXPORT_SYMBOL(_GPL)?\s*\(\s*(\w+)\s*\)\s*;';
> -my $export_symbol_ns = '^\s*EXPORT_SYMBOL_NS(_GPL)?\s*\(\s*(\w+)\s*,\s*\w+\)\s*;';
> +my $export_symbol_ns = '^\s*EXPORT_SYMBOL_NS(_GPL)?\s*\(\s*(\w+)\s*,\s*"\S+"\)\s*;';

Could we simplify this?  We don't seem to treat export_symbol and
export_symbol_ns differently, so why not catch all possibilities with
one regex?

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index f66070176ba3..de22f88f7a35 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -266,8 +266,7 @@ my $doc_inline_start = '^\s*/\*\*\s*$';
 my $doc_inline_sect = '\s*\*\s*(@\s*[\w][\w\.]*\s*):(.*)';
 my $doc_inline_end = '^\s*\*/\s*$';
 my $doc_inline_oneline = '^\s*/\*\*\s*(@[\w\s]+):\s*(.*)\s*\*/\s*$';
-my $export_symbol = '^\s*EXPORT_SYMBOL(_GPL)?\s*\(\s*(\w+)\s*\)\s*;';
-my $export_symbol_ns = '^\s*EXPORT_SYMBOL_NS(_GPL)?\s*\(\s*(\w+)\s*,\s*\w+\)\s*;';
+my $export_symbol = '^\s*EXPORT_SYMBOL[_A-Z]*\s*\(\s*(\w+)\s*\)\s*;';
 my $function_pointer = qr{([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)};
 my $attribute = qr{__attribute__\s*\(\([a-z0-9,_\*\s\(\)]*\)\)}i;
 
@@ -2024,12 +2023,8 @@ sub process_export_file($) {
 
     while (<IN>) {
         if (/$export_symbol/) {
-            next if (defined($nosymbol_table{$2}));
-            $function_table{$2} = 1;
-        }
-        if (/$export_symbol_ns/) {
-            next if (defined($nosymbol_table{$2}));
-            $function_table{$2} = 1;
+            next if (defined($nosymbol_table{$1}));
+            $function_table{$1} = 1;
         }
     }
 

Re: [PATCH] scripts/kernel-doc: Get -export option working again

[Jonathan Corbet]

Akira Yokosawa <akiyks@gmail.com> writes:

> Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
> literal"), exported symbols marked by EXPORT_SYMBOL_NS(_GPL) are
> ignored by "kernel-doc -export" in fresh build of "make htmldocs".
>
> This is because regex in the perl script for those markers fails to
> match the new signatures:
>
>   - EXPORT_SYMBOL_NS(symbol, "ns");
>   - EXPORT_SYMBOL_NS_GPL(symbol, "ns");
>
> Update the regex so that it matches quoted string.
>
> Note: Escape sequence of \w is good for C identifiers, but can be
> too strict for quoted strings.  Instead, use \S, which matches any
> non-whitespace character, for compatibility with possible extension
> of namespace convention in the future [1].
>
> Fixes: cdd30ebb1b9f ("module: Convert symbol namespace to string literal")
> Link: https://lore.kernel.org/CAK7LNATMufXP0EA6QUE9hBkZMa6vJO6ZiaYuak2AhOrd2nSVKQ@mail.gmail.com/ [1]
> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
> Cc: Masahiro Yamada <masahiroy@kernel.org>

Ah ... I should have thought of that ... I'm glad you did.  I've just
pushed the fix out to linux-next, will send it Linusward in the near
future.  Thanks for fixing this,

jon

Re: [PATCH] scripts/kernel-doc: Get -export option working again

[Akira Yokosawa]

Jonathan Corbet wrote:
[...]
> Ah ... I should have thought of that ... I'm glad you did.  I've just
> pushed the fix out to linux-next, will send it Linusward in the near
> future.  Thanks for fixing this,

Your docs-fixes is based on v6.13-rc1.
This fix needs to come after v6.13-rc2.

On current docs-next,

    ./script/kernel-doc -rst -export drivers/iommu/iommufd/device.c

returns:

    drivers/iommu/iommufd/device.c:1: warning: no structured comments found

I guess restarting everything from v6.13-rc2 would make our life much easier
for v6.14.

        Thanks, Akira

Re: [PATCH] scripts/kernel-doc: Get -export option working again

[Jonathan Corbet]

Akira Yokosawa <akiyks@gmail.com> writes:

> Jonathan Corbet wrote:
> [...]
>> Ah ... I should have thought of that ... I'm glad you did.  I've just
>> pushed the fix out to linux-next, will send it Linusward in the near
>> future.  Thanks for fixing this,
>
> Your docs-fixes is based on v6.13-rc1.
> This fix needs to come after v6.13-rc2.

...which will happen once the fix hits mainline - the *fix* doesn't
depend on -rc2.

> On current docs-next,
>
>     ./script/kernel-doc -rst -export drivers/iommu/iommufd/device.c
>
> returns:
>
>     drivers/iommu/iommufd/device.c:1: warning: no structured comments found
>
> I guess restarting everything from v6.13-rc2 would make our life much easier
> for v6.14.

I suppose I can rebase docs-next forward (or back-merge) if there's a
real need to do so... I'm not sure how important that is to get rid of a
few warnings?

Thanks,

jon

Re: [PATCH] scripts/kernel-doc: Get -export option working again

[Akira Yokosawa]

Jonathan Corbet wrote:
> Akira Yokosawa <akiyks@gmail.com> writes:
> 
>> Jonathan Corbet wrote:
>> [...]
>>> Ah ... I should have thought of that ... I'm glad you did.  I've just
>>> pushed the fix out to linux-next, will send it Linusward in the near
>>> future.  Thanks for fixing this,
>>
>> Your docs-fixes is based on v6.13-rc1.
>> This fix needs to come after v6.13-rc2.
> 
> ...which will happen once the fix hits mainline - the *fix* doesn't
> depend on -rc2.
> 

Well...
The fix conflicts semantically against v6.13-rc1.

I know you hate full rebuild of htmldocs. But I'd like you to follow
the following steps to see the full scope of this fix.

 - git checkout v6.13-rc2
 - git checkout docs-fixes
 - make htmldocs

You might be surprised, but you'll see the following:

./drivers/dma-buf/dma-buf.c:1: warning: no structured comments found
./drivers/iommu/iommufd/device.c:1: warning: no structured comments found
./drivers/iommu/iommufd/main.c:1: warning: no structured comments found
./drivers/counter/counter-core.c:1: warning: no structured comments found
./drivers/counter/counter-chrdev.c:1: warning: no structured comments found

Current docs-fixes has a semantic conflicts opposite to the one v6.13-rc2 has.

I'd really like you to send a pull request that resolves the semantic
conflict existing in v6.13-rc2, not the one that happens to negate the
conflict on merge.

That's the minimal expectation I have.

You might wonder whey you need to checkout v6.13-rc2 first and go back
to docs-fixes to see the conflict there.

That's because "make htmldocs" don't rerun kernel-doc conversion
when only the kernel-doc script is updated.  To see the effect of
its change, source files with affected kernel-doc comments and
"EXPORT_SYMBOL*" need to be updated as well.

Of course you are well aware of this problematic behavior of
documentation build.

I don't mind docs-mw if you'd like to keep it v6.13-rc1 based.

Have I made my points clear enough for you?

        Thanks, Akira

Re: [PATCH] scripts/kernel-doc: Get -export option working again

[Jonathan Corbet]

Akira Yokosawa <akiyks@gmail.com> writes:

> Jonathan Corbet wrote:
>> Akira Yokosawa <akiyks@gmail.com> writes:
>> 
>>> Jonathan Corbet wrote:
>>> [...]
>>>> Ah ... I should have thought of that ... I'm glad you did.  I've just
>>>> pushed the fix out to linux-next, will send it Linusward in the near
>>>> future.  Thanks for fixing this,
>>>
>>> Your docs-fixes is based on v6.13-rc1.
>>> This fix needs to come after v6.13-rc2.
>> 
>> ...which will happen once the fix hits mainline - the *fix* doesn't
>> depend on -rc2.
>> 
>
> Well...
> The fix conflicts semantically against v6.13-rc1.
>
> I know you hate full rebuild of htmldocs. But I'd like you to follow
> the following steps to see the full scope of this fix.
>
>  - git checkout v6.13-rc2
>  - git checkout docs-fixes
>  - make htmldocs
>
> You might be surprised, but you'll see the following:
>
> ./drivers/dma-buf/dma-buf.c:1: warning: no structured comments found
> ./drivers/iommu/iommufd/device.c:1: warning: no structured comments found
> ./drivers/iommu/iommufd/main.c:1: warning: no structured comments found
> ./drivers/counter/counter-core.c:1: warning: no structured comments found
> ./drivers/counter/counter-chrdev.c:1: warning: no structured comments found

So I guess I don't really see the problem - nobody is going to do that
(except seemingly you :)

> Current docs-fixes has a semantic conflicts opposite to the one v6.13-rc2 has.
>
> I'd really like you to send a pull request that resolves the semantic
> conflict existing in v6.13-rc2, not the one that happens to negate the
> conflict on merge.
>
> That's the minimal expectation I have.
>
> You might wonder whey you need to checkout v6.13-rc2 first and go back
> to docs-fixes to see the conflict there.
>
> That's because "make htmldocs" don't rerun kernel-doc conversion
> when only the kernel-doc script is updated.  To see the effect of
> its change, source files with affected kernel-doc comments and
> "EXPORT_SYMBOL*" need to be updated as well.
>
> Of course you are well aware of this problematic behavior of
> documentation build.

...which is worth fixing in the build system...

> I don't mind docs-mw if you'd like to keep it v6.13-rc1 based.
>
> Have I made my points clear enough for you?

I don't fully understand the problem you are concerned about, no.  But
it doesn't really matter that much, I'd rather not have an important
contributor being unhappy with me.  So I've just pulled everything
forward to -rc2 and force-pushed it; hopefully things are better now?

I'll probably send the fix upstream on Friday.

Thanks,

jon