[PATCH 1/2] doc: module: Fix documented type of namespace

Uwe Kleine-König posted 2 patches 1 year ago
There is a newer version of this series
[PATCH 1/2] doc: module: Fix documented type of namespace
Posted by Uwe Kleine-König 1 year ago
Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
literal") the namespace has to be a string. Fix accordingly.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 Documentation/core-api/symbol-namespaces.rst | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
index 27a9cccc792c..a08a3448cbad 100644
--- a/Documentation/core-api/symbol-namespaces.rst
+++ b/Documentation/core-api/symbol-namespaces.rst
@@ -41,9 +41,8 @@ entries.
 In addition to the macros EXPORT_SYMBOL() and EXPORT_SYMBOL_GPL(), that allow
 exporting of kernel symbols to the kernel symbol table, variants of these are
 available to export symbols into a certain namespace: EXPORT_SYMBOL_NS() and
-EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace.
-Please note that due to macro expansion that argument needs to be a
-preprocessor symbol. E.g. to export the symbol ``usb_stor_suspend`` into the
+EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace as a
+C-string. E.g. to export the symbol ``usb_stor_suspend`` into the
 namespace ``USB_STORAGE``, use::
 
 	EXPORT_SYMBOL_NS(usb_stor_suspend, "USB_STORAGE");
-- 
2.45.2

Re: [PATCH 1/2] doc: module: Fix documented type of namespace
Posted by Andy Shevchenko 1 year ago
On Wed, Dec 04, 2024 at 11:01:10AM +0100, Uwe Kleine-König wrote:
> Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
> literal") the namespace has to be a string. Fix accordingly.

>  In addition to the macros EXPORT_SYMBOL() and EXPORT_SYMBOL_GPL(), that allow
>  exporting of kernel symbols to the kernel symbol table, variants of these are
>  available to export symbols into a certain namespace: EXPORT_SYMBOL_NS() and
> -EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace.
> -Please note that due to macro expansion that argument needs to be a
> -preprocessor symbol. E.g. to export the symbol ``usb_stor_suspend`` into the
> +EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace as a
> +C-string. E.g. to export the symbol ``usb_stor_suspend`` into the

But C-string is ambiguous. Is it now okay to use

static const char *p = "my constant C-string";

EXPORT_...(, p);

?

>  namespace ``USB_STORAGE``, use::

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 1/2] doc: module: Fix documented type of namespace
Posted by Uwe Kleine-König 1 year ago
On Thu, Dec 05, 2024 at 09:50:18AM +0200, Andy Shevchenko wrote:
> On Wed, Dec 04, 2024 at 11:01:10AM +0100, Uwe Kleine-König wrote:
> > Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
> > literal") the namespace has to be a string. Fix accordingly.
> 
> >  In addition to the macros EXPORT_SYMBOL() and EXPORT_SYMBOL_GPL(), that allow
> >  exporting of kernel symbols to the kernel symbol table, variants of these are
> >  available to export symbols into a certain namespace: EXPORT_SYMBOL_NS() and
> > -EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace.
> > -Please note that due to macro expansion that argument needs to be a
> > -preprocessor symbol. E.g. to export the symbol ``usb_stor_suspend`` into the
> > +EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace as a
> > +C-string. E.g. to export the symbol ``usb_stor_suspend`` into the
> 
> But C-string is ambiguous. Is it now okay to use
> 
> static const char *p = "my constant C-string";
> 
> EXPORT_...(, p);

I didn't test that, but I guess that won't work. While you could argue
that p isn't a C-string but a pointer, I agree that the wording isn't
optimal.

So maybe make that:

	... the namespace as a string constant.

?

Best regards
Uwe
Re: [PATCH 1/2] doc: module: Fix documented type of namespace
Posted by Andy Shevchenko 1 year ago
On Thu, Dec 05, 2024 at 11:55:54AM +0100, Uwe Kleine-König wrote:
> On Thu, Dec 05, 2024 at 09:50:18AM +0200, Andy Shevchenko wrote:
> > On Wed, Dec 04, 2024 at 11:01:10AM +0100, Uwe Kleine-König wrote:
> > > Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
> > > literal") the namespace has to be a string. Fix accordingly.
> > 
> > >  In addition to the macros EXPORT_SYMBOL() and EXPORT_SYMBOL_GPL(), that allow
> > >  exporting of kernel symbols to the kernel symbol table, variants of these are
> > >  available to export symbols into a certain namespace: EXPORT_SYMBOL_NS() and
> > > -EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace.
> > > -Please note that due to macro expansion that argument needs to be a
> > > -preprocessor symbol. E.g. to export the symbol ``usb_stor_suspend`` into the
> > > +EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace as a
> > > +C-string. E.g. to export the symbol ``usb_stor_suspend`` into the
> > 
> > But C-string is ambiguous. Is it now okay to use
> > 
> > static const char *p = "my constant C-string";
> > 
> > EXPORT_...(, p);
> 
> I didn't test that, but I guess that won't work. While you could argue
> that p isn't a C-string but a pointer, I agree that the wording isn't
> optimal.
> 
> So maybe make that:
> 
> 	... the namespace as a string constant.

...a string literal.

?

> ?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 1/2] doc: module: Fix documented type of namespace
Posted by Uwe Kleine-König 1 year ago
On Thu, Dec 05, 2024 at 09:04:50PM +0200, Andy Shevchenko wrote:
> On Thu, Dec 05, 2024 at 11:55:54AM +0100, Uwe Kleine-König wrote:
> > On Thu, Dec 05, 2024 at 09:50:18AM +0200, Andy Shevchenko wrote:
> > > On Wed, Dec 04, 2024 at 11:01:10AM +0100, Uwe Kleine-König wrote:
> > > > Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
> > > > literal") the namespace has to be a string. Fix accordingly.
> > > 
> > > >  In addition to the macros EXPORT_SYMBOL() and EXPORT_SYMBOL_GPL(), that allow
> > > >  exporting of kernel symbols to the kernel symbol table, variants of these are
> > > >  available to export symbols into a certain namespace: EXPORT_SYMBOL_NS() and
> > > > -EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace.
> > > > -Please note that due to macro expansion that argument needs to be a
> > > > -preprocessor symbol. E.g. to export the symbol ``usb_stor_suspend`` into the
> > > > +EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace as a
> > > > +C-string. E.g. to export the symbol ``usb_stor_suspend`` into the
> > > 
> > > But C-string is ambiguous. Is it now okay to use
> > > 
> > > static const char *p = "my constant C-string";
> > > 
> > > EXPORT_...(, p);
> > 
> > I didn't test that, but I guess that won't work. While you could argue
> > that p isn't a C-string but a pointer, I agree that the wording isn't
> > optimal.
> > 
> > So maybe make that:
> > 
> > 	... the namespace as a string constant.
> 
> ...a string literal.

Gcc calls it "string constant":
https://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#String-Constants

My C book (https://www.amazon.de/dp/013089592X) also calls it "string
constant".

So I tend to keep that name as it seems to be the official term.

Best regards
Uwe
Re: [PATCH 1/2] doc: module: Fix documented type of namespace
Posted by Masahiro Yamada 11 months, 3 weeks ago
On Fri, Dec 6, 2024 at 11:46 PM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> On Thu, Dec 05, 2024 at 09:04:50PM +0200, Andy Shevchenko wrote:
> > On Thu, Dec 05, 2024 at 11:55:54AM +0100, Uwe Kleine-König wrote:
> > > On Thu, Dec 05, 2024 at 09:50:18AM +0200, Andy Shevchenko wrote:
> > > > On Wed, Dec 04, 2024 at 11:01:10AM +0100, Uwe Kleine-König wrote:
> > > > > Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
> > > > > literal") the namespace has to be a string. Fix accordingly.
> > > >
> > > > >  In addition to the macros EXPORT_SYMBOL() and EXPORT_SYMBOL_GPL(), that allow
> > > > >  exporting of kernel symbols to the kernel symbol table, variants of these are
> > > > >  available to export symbols into a certain namespace: EXPORT_SYMBOL_NS() and
> > > > > -EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace.
> > > > > -Please note that due to macro expansion that argument needs to be a
> > > > > -preprocessor symbol. E.g. to export the symbol ``usb_stor_suspend`` into the
> > > > > +EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace as a
> > > > > +C-string. E.g. to export the symbol ``usb_stor_suspend`` into the
> > > >
> > > > But C-string is ambiguous. Is it now okay to use
> > > >
> > > > static const char *p = "my constant C-string";
> > > >
> > > > EXPORT_...(, p);
> > >
> > > I didn't test that, but I guess that won't work. While you could argue
> > > that p isn't a C-string but a pointer, I agree that the wording isn't
> > > optimal.
> > >
> > > So maybe make that:
> > >
> > >     ... the namespace as a string constant.
> >
> > ...a string literal.
>
> Gcc calls it "string constant":
> https://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#String-Constants
>
> My C book (https://www.amazon.de/dp/013089592X) also calls it "string
> constant".
>
> So I tend to keep that name as it seems to be the official term.


But, you do not call it a "C-string".

EXPORT_SYMBOL_NS() is expanded into assembly code.

See:
        .asciz ns


So, will you send v2 with "string constant" or "string literal"?
(I think either is fine.)



-- 
Best Regards
Masahiro Yamada
Re: [PATCH 1/2] doc: module: Fix documented type of namespace
Posted by Andy Shevchenko 1 year ago
On Fri, Dec 06, 2024 at 03:45:50PM +0100, Uwe Kleine-König wrote:
> On Thu, Dec 05, 2024 at 09:04:50PM +0200, Andy Shevchenko wrote:
> > On Thu, Dec 05, 2024 at 11:55:54AM +0100, Uwe Kleine-König wrote:
> > > On Thu, Dec 05, 2024 at 09:50:18AM +0200, Andy Shevchenko wrote:
> > > > On Wed, Dec 04, 2024 at 11:01:10AM +0100, Uwe Kleine-König wrote:

...

> > > > > Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
> > > > > literal") the namespace has to be a string. Fix accordingly.

^^^ (1)

...

> > > 	... the namespace as a string constant.
> > 
> > ...a string literal.
> 
> Gcc calls it "string constant":
> https://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#String-Constants
> 
> My C book (https://www.amazon.de/dp/013089592X) also calls it "string
> constant".
> 
> So I tend to keep that name as it seems to be the official term.

Even though we should be more consistent with the de facto usages, no?

(see (1) above, for example).

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 1/2] doc: module: Fix documented type of namespace
Posted by Jani Nikula 1 year ago
On Wed, 04 Dec 2024, Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote:
> Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
> literal") the namespace has to be a string. Fix accordingly.

Interesting. Using preprocessor symbols inherently restricted the
namespace syntax to that of C identifiers.

Is it now okay to use any syntax for the namespaces from now on? Should
the document have some recommendations for naming namespaces?

BR,
Jani.

>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
>  Documentation/core-api/symbol-namespaces.rst | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
> index 27a9cccc792c..a08a3448cbad 100644
> --- a/Documentation/core-api/symbol-namespaces.rst
> +++ b/Documentation/core-api/symbol-namespaces.rst
> @@ -41,9 +41,8 @@ entries.
>  In addition to the macros EXPORT_SYMBOL() and EXPORT_SYMBOL_GPL(), that allow
>  exporting of kernel symbols to the kernel symbol table, variants of these are
>  available to export symbols into a certain namespace: EXPORT_SYMBOL_NS() and
> -EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace.
> -Please note that due to macro expansion that argument needs to be a
> -preprocessor symbol. E.g. to export the symbol ``usb_stor_suspend`` into the
> +EXPORT_SYMBOL_NS_GPL(). They take one additional argument: the namespace as a
> +C-string. E.g. to export the symbol ``usb_stor_suspend`` into the
>  namespace ``USB_STORAGE``, use::
>  
>  	EXPORT_SYMBOL_NS(usb_stor_suspend, "USB_STORAGE");

-- 
Jani Nikula, Intel
Re: [PATCH 1/2] doc: module: Fix documented type of namespace
Posted by Masahiro Yamada 1 year ago
On Wed, Dec 4, 2024 at 7:38 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Wed, 04 Dec 2024, Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote:
> > Since commit cdd30ebb1b9f ("module: Convert symbol namespace to string
> > literal") the namespace has to be a string. Fix accordingly.
>
> Interesting. Using preprocessor symbols inherently restricted the
> namespace syntax to that of C identifiers.
>
> Is it now okay to use any syntax for the namespaces from now on? Should
> the document have some recommendations for naming namespaces?

I guess the answer is yes except for whitespaces.

The namespaces are recorded in the Module.symvers file,
which is also parsed by external programs like kmod.
Using whitespaces within namespaces may confuse the field separators.
Otherwise, the namespace is a simple string matching as far as I can tell.






-- 
Best Regards
Masahiro Yamada