[PATCH 0/2] doc: module: Two clearifications

[PATCH 0/2] doc: module: Two clearifications

[Uwe Kleine-König]

Hello,

here come to fixes for the symbol namespace docs around
DEFAULT_SYMBOL_NAMESPACE. The first updates the text to say that a
namespace has to be a C-string. Before it claimed it should be a
preprocessor symbol. I'm not sure how to understand it, but today it's
wrong for sure.

Fixes a statement that Andy pointed out to be wrong in the docs while
discussing the respective fix in drivers/i2c/busses/i2c-designware-*.
(See
https://lore.kernel.org/linux-i2c/20241203173640.1648939-2-u.kleine-koenig@baylibre.com)

Uwe Kleine-König (2):
  doc: module: Fix documented type of namespace
  doc: module: DEFAULT_SYMBOL_NAMESPACE must be defined before #includes

 Documentation/core-api/symbol-namespaces.rst | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)


base-commit: ceb8bf2ceaa77fe222fe8fe32cb7789c9099ddf1
-- 
2.45.2

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

[Uwe Kleine-König]

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

[Jani Nikula]

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

[Masahiro Yamada]

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

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

[Andy Shevchenko]

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1307 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Andy Shevchenko]

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1839 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Andy Shevchenko]

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

[Masahiro Yamada]

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

[PATCH 2/2] doc: module: DEFAULT_SYMBOL_NAMESPACE must be defined before #includes

[Uwe Kleine-König]

The definition of EXPORT_SYMBOL et al depends on
DEFAULT_SYMBOL_NAMESPACE. So DEFAULT_SYMBOL_NAMESPACE must already be
available when <linux/export.h> is parsed.

Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/linux-i2c/Z09bp9uMzwXRLXuF@smile.fi.intel.com/
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 Documentation/core-api/symbol-namespaces.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
index a08a3448cbad..d63e4245a9f5 100644
--- a/Documentation/core-api/symbol-namespaces.rst
+++ b/Documentation/core-api/symbol-namespaces.rst
@@ -80,8 +80,8 @@ unit as preprocessor statement. The above example would then read::
 	#undef  DEFAULT_SYMBOL_NAMESPACE
 	#define DEFAULT_SYMBOL_NAMESPACE "USB_COMMON"
 
-within the corresponding compilation unit before any EXPORT_SYMBOL macro is
-used.
+within the corresponding compilation unit before the #include for
+<linux/export.h>.
 
 3. How to use Symbols exported in Namespaces
 ============================================
-- 
2.45.2

Re: [PATCH 2/2] doc: module: DEFAULT_SYMBOL_NAMESPACE must be defined before #includes

[Andy Shevchenko]

On Wed, Dec 04, 2024 at 11:01:11AM +0100, Uwe Kleine-König wrote:
> The definition of EXPORT_SYMBOL et al depends on
> DEFAULT_SYMBOL_NAMESPACE. So DEFAULT_SYMBOL_NAMESPACE must already be
> available when <linux/export.h> is parsed.

> -within the corresponding compilation unit before any EXPORT_SYMBOL macro is
> -used.
> +within the corresponding compilation unit before the #include for
> +<linux/export.h>.

And how do I know where it is included in the current state of affairs with the
dependency hell in the kernel?

I suggest to reword it to something like "before any global inclusions", the
best probably is to repeat the piece from pr_fmt() / dev_fmt() documentation
(if any).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] doc: module: DEFAULT_SYMBOL_NAMESPACE must be defined before #includes

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1158 bytes --]

Hello Andy,

On Thu, Dec 05, 2024 at 09:52:13AM +0200, Andy Shevchenko wrote:
> On Wed, Dec 04, 2024 at 11:01:11AM +0100, Uwe Kleine-König wrote:
> > The definition of EXPORT_SYMBOL et al depends on
> > DEFAULT_SYMBOL_NAMESPACE. So DEFAULT_SYMBOL_NAMESPACE must already be
> > available when <linux/export.h> is parsed.
> 
> > -within the corresponding compilation unit before any EXPORT_SYMBOL macro is
> > -used.
> > +within the corresponding compilation unit before the #include for
> > +<linux/export.h>.
> 
> And how do I know where it is included in the current state of affairs with the
> dependency hell in the kernel?
> 
> I suggest to reword it to something like "before any global inclusions", the
> best probably is to repeat the piece from pr_fmt() / dev_fmt() documentation
> (if any).

Well, "before <linux/export.h>" is the accurate thing you have to
ensure. "before any global inclusion" is the safe and easy thing to do
to achieve that. Maybe:

	...
	within the corresponding compilation unit before the #include for
	<linux/export.h>. Typically it's placed before the first
	#include.

?

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] doc: module: DEFAULT_SYMBOL_NAMESPACE must be defined before #includes

[Andy Shevchenko]

On Thu, Dec 05, 2024 at 12:21:26PM +0100, Uwe Kleine-König wrote:
> Hello Andy,
> 
> On Thu, Dec 05, 2024 at 09:52:13AM +0200, Andy Shevchenko wrote:
> > On Wed, Dec 04, 2024 at 11:01:11AM +0100, Uwe Kleine-König wrote:
> > > The definition of EXPORT_SYMBOL et al depends on
> > > DEFAULT_SYMBOL_NAMESPACE. So DEFAULT_SYMBOL_NAMESPACE must already be
> > > available when <linux/export.h> is parsed.
> > 
> > > -within the corresponding compilation unit before any EXPORT_SYMBOL macro is
> > > -used.
> > > +within the corresponding compilation unit before the #include for
> > > +<linux/export.h>.
> > 
> > And how do I know where it is included in the current state of affairs with the
> > dependency hell in the kernel?
> > 
> > I suggest to reword it to something like "before any global inclusions", the
> > best probably is to repeat the piece from pr_fmt() / dev_fmt() documentation
> > (if any).
> 
> Well, "before <linux/export.h>" is the accurate thing you have to
> ensure. "before any global inclusion" is the safe and easy thing to do
> to achieve that. Maybe:
> 
> 	...
> 	within the corresponding compilation unit before the #include for
> 	<linux/export.h>. Typically it's placed before the first
> 	#include.

The below is excerpt for pr_fmt():

---8>----
Besides being more concise than the equivalent printk() calls, they can use a
common definition for the format string through the pr_fmt() macro. For
instance, defining this at the top of a source file (before any ``#include``
directive)::

  #define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__

---8>----

> ?


-- 
With Best Regards,
Andy Shevchenko