[PATCH 0/2] string.c: Add wcslen()

[PATCH 0/2] string.c: Add wcslen()

[Nathan Chancellor]

Hi all,

A recent LLVM change [1] introduces a call to wcslen() in
fs/smb/client/smb2pdu.c through UniStrcat() via
alloc_path_with_tree_prefix(). Similar to the bcmp() and stpcpy()
additions that happened in 5f074f3e192f and 1e1b6d63d634, add wcslen()
to fix the linkage failure.

The second change is RFC because it makes the first change a little more
convoluted for the sake of making it externally available, which may or
may not be desirable. See the commit message for more details.

[1]: https://github.com/llvm/llvm-project/commit/9694844d7e36fd5e01011ab56b64f27b867aa72d

---
Nathan Chancellor (2):
      lib/string.c: Add wcslen()
      [RFC] wcslen() prototype in string.h

 drivers/firmware/efi/libstub/printk.c |  4 ++--
 include/linux/string.h                |  2 ++
 lib/string.c                          | 11 +++++++++++
 3 files changed, 15 insertions(+), 2 deletions(-)
---
base-commit: 78ab93c78fb31c5dfe207318aa2b7bd4e41f8dba
change-id: 20250324-string-add-wcslen-for-llvm-opt-705791db92c0

Best regards,
-- 
Nathan Chancellor <nathan@kernel.org>

[PATCH 1/2] lib/string.c: Add wcslen()

[Nathan Chancellor]

A recent optimization change in LLVM [1] aims to transform certain loop
idioms into calls to strlen() or wcslen(). This change transforms the
first while loop in UniStrcat() into a call to wcslen(), breaking the
build when UniStrcat() gets inlined into alloc_path_with_tree_prefix():

  ld.lld: error: undefined symbol: wcslen
  >>> referenced by nls_ucs2_utils.h:54 (fs/smb/client/../../nls/nls_ucs2_utils.h:54)
  >>>               vmlinux.o:(alloc_path_with_tree_prefix)
  >>> referenced by nls_ucs2_utils.h:54 (fs/smb/client/../../nls/nls_ucs2_utils.h:54)
  >>>               vmlinux.o:(alloc_path_with_tree_prefix)

The kernel does not build with '-ffreestanding' (which would avoid this
transformation) because it does want libcall optimizations in general
and turning on '-ffreestanding' disables the majority of them. While
'-fno-builtin-wcslen' would be more targeted at the problem, it does not
work with LTO.

Add a basic wcslen() to avoid this linkage failure. While no
architecture or FORTIFY_SOURCE overrides this, add it to string.c
instead of string_helpers.c so that it is built with '-ffreestanding',
otherwise the compiler might transform it into a call to itself. Give
wcslen() an internal prototype to avoid -Wmissing-prototypes (a global
prototype needs a little extra work and it is not currently needed).

Cc: stable@vger.kernel.org
Link: https://github.com/llvm/llvm-project/commit/9694844d7e36fd5e01011ab56b64f27b867aa72d [1]
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 lib/string.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/string.c b/lib/string.c
index eb4486ed40d2..bbee8a9e4d83 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -21,6 +21,7 @@
 #include <linux/errno.h>
 #include <linux/limits.h>
 #include <linux/linkage.h>
+#include <linux/nls.h>
 #include <linux/stddef.h>
 #include <linux/string.h>
 #include <linux/types.h>
@@ -429,6 +430,17 @@ size_t strnlen(const char *s, size_t count)
 EXPORT_SYMBOL(strnlen);
 #endif
 
+size_t wcslen(const wchar_t *s);
+size_t wcslen(const wchar_t *s)
+{
+	const wchar_t *sc;
+
+	for (sc = s; *sc != '\0'; ++sc)
+		/* nothing */;
+	return sc - s;
+}
+EXPORT_SYMBOL(wcslen);
+
 #ifndef __HAVE_ARCH_STRSPN
 /**
  * strspn - Calculate the length of the initial substring of @s which only contain letters in @accept

-- 
2.49.0

[PATCH RFC 2/2] wcslen() prototype in string.h

[Nathan Chancellor]

If this is desired, it should be squashed into the previous change. I
wrote it separately because it is slightly more invasive.

In order to export wcslen() to the rest of the kernel (should it ever be
necessary elsewhere), it needs to be added to string.h, along with nls.h
for the typedef of wchar_t. However, dragging in nls.h into string.h
causes an error in the efistub due to a conflicting function name:

  drivers/firmware/efi/libstub/printk.c:27:5: error: static declaration of 'utf8_to_utf32' follows non-static declaration
     27 | u32 utf8_to_utf32(const u8 **s8)
        |     ^
  include/linux/nls.h:55:12: note: previous declaration is here
     55 | extern int utf8_to_utf32(const u8 *s, int len, unicode_t *pu);
        |            ^
  drivers/firmware/efi/libstub/printk.c:85:26: error: too few arguments to function call, expected 3, have 1
     85 |                 c32 = utf8_to_utf32(&s8);
        |                       ~~~~~~~~~~~~~    ^
  include/linux/nls.h:55:12: note: 'utf8_to_utf32' declared here
     55 | extern int utf8_to_utf32(const u8 *s, int len, unicode_t *pu);
        |            ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  2 errors generated.

Rename the efi function to avoid the conflict.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/firmware/efi/libstub/printk.c | 4 ++--
 include/linux/string.h                | 2 ++
 lib/string.c                          | 1 -
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/libstub/printk.c b/drivers/firmware/efi/libstub/printk.c
index 3a67a2cea7bd..334f7e89845c 100644
--- a/drivers/firmware/efi/libstub/printk.c
+++ b/drivers/firmware/efi/libstub/printk.c
@@ -24,7 +24,7 @@ void efi_char16_puts(efi_char16_t *str)
 }
 
 static
-u32 utf8_to_utf32(const u8 **s8)
+u32 efi_utf8_to_utf32(const u8 **s8)
 {
 	u32 c32;
 	u8 c0, cx;
@@ -82,7 +82,7 @@ void efi_puts(const char *str)
 	while (*s8) {
 		if (*s8 == '\n')
 			buf[pos++] = L'\r';
-		c32 = utf8_to_utf32(&s8);
+		c32 = efi_utf8_to_utf32(&s8);
 		if (c32 < 0x10000) {
 			/* Characters in plane 0 use a single word. */
 			buf[pos++] = c32;
diff --git a/include/linux/string.h b/include/linux/string.h
index 0403a4ca4c11..45e01cf3434c 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -7,6 +7,7 @@
 #include <linux/cleanup.h>	/* for DEFINE_FREE() */
 #include <linux/compiler.h>	/* for inline */
 #include <linux/types.h>	/* for size_t */
+#include <linux/nls.h>		/* for wchar_t */
 #include <linux/stddef.h>	/* for NULL */
 #include <linux/err.h>		/* for ERR_PTR() */
 #include <linux/errno.h>	/* for E2BIG */
@@ -203,6 +204,7 @@ extern __kernel_size_t strlen(const char *);
 #ifndef __HAVE_ARCH_STRNLEN
 extern __kernel_size_t strnlen(const char *,__kernel_size_t);
 #endif
+extern __kernel_size_t wcslen(const wchar_t *s);
 #ifndef __HAVE_ARCH_STRPBRK
 extern char * strpbrk(const char *,const char *);
 #endif
diff --git a/lib/string.c b/lib/string.c
index bbee8a9e4d83..1aa09925254b 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -430,7 +430,6 @@ size_t strnlen(const char *s, size_t count)
 EXPORT_SYMBOL(strnlen);
 #endif
 
-size_t wcslen(const wchar_t *s);
 size_t wcslen(const wchar_t *s)
 {
 	const wchar_t *sc;

-- 
2.49.0

Re: [PATCH RFC 2/2] wcslen() prototype in string.h

[Andy Shevchenko]

On Tue, Mar 25, 2025 at 08:45:19AM -0700, Nathan Chancellor wrote:
> If this is desired, it should be squashed into the previous change. I
> wrote it separately because it is slightly more invasive.
> 
> In order to export wcslen() to the rest of the kernel (should it ever be
> necessary elsewhere), it needs to be added to string.h, along with nls.h
> for the typedef of wchar_t. However, dragging in nls.h into string.h
> causes an error in the efistub due to a conflicting function name:
> 
>   drivers/firmware/efi/libstub/printk.c:27:5: error: static declaration of 'utf8_to_utf32' follows non-static declaration
>      27 | u32 utf8_to_utf32(const u8 **s8)
>         |     ^
>   include/linux/nls.h:55:12: note: previous declaration is here
>      55 | extern int utf8_to_utf32(const u8 *s, int len, unicode_t *pu);
>         |            ^
>   drivers/firmware/efi/libstub/printk.c:85:26: error: too few arguments to function call, expected 3, have 1
>      85 |                 c32 = utf8_to_utf32(&s8);
>         |                       ~~~~~~~~~~~~~    ^
>   include/linux/nls.h:55:12: note: 'utf8_to_utf32' declared here
>      55 | extern int utf8_to_utf32(const u8 *s, int len, unicode_t *pu);
>         |            ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   2 errors generated.
> 
> Rename the efi function to avoid the conflict.

Hmm... Why not split this to two, rename patch as a standalone makes sense to
me even outside of this series.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RFC 2/2] wcslen() prototype in string.h

[Nathan Chancellor]

On Tue, Mar 25, 2025 at 06:17:34PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 25, 2025 at 08:45:19AM -0700, Nathan Chancellor wrote:
> > If this is desired, it should be squashed into the previous change. I
> > wrote it separately because it is slightly more invasive.
> > 
> > In order to export wcslen() to the rest of the kernel (should it ever be
> > necessary elsewhere), it needs to be added to string.h, along with nls.h
> > for the typedef of wchar_t. However, dragging in nls.h into string.h
> > causes an error in the efistub due to a conflicting function name:
> > 
> >   drivers/firmware/efi/libstub/printk.c:27:5: error: static declaration of 'utf8_to_utf32' follows non-static declaration
> >      27 | u32 utf8_to_utf32(const u8 **s8)
> >         |     ^
> >   include/linux/nls.h:55:12: note: previous declaration is here
> >      55 | extern int utf8_to_utf32(const u8 *s, int len, unicode_t *pu);
> >         |            ^
> >   drivers/firmware/efi/libstub/printk.c:85:26: error: too few arguments to function call, expected 3, have 1
> >      85 |                 c32 = utf8_to_utf32(&s8);
> >         |                       ~~~~~~~~~~~~~    ^
> >   include/linux/nls.h:55:12: note: 'utf8_to_utf32' declared here
> >      55 | extern int utf8_to_utf32(const u8 *s, int len, unicode_t *pu);
> >         |            ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   2 errors generated.
> > 
> > Rename the efi function to avoid the conflict.
> 
> Hmm... Why not split this to two, rename patch as a standalone makes sense to
> me even outside of this series.

How so? If nls.h is not included in printk.c via string.h, which does
not happen without this series, what value does the rename have? I do
not mind splitting it up that way to keep things cleaner, I am just
wondering what would be the justification in the changelog (I guess just
that nls.h may get included in the future for some reason)?

Cheers,
Nathan

Re: [PATCH RFC 2/2] wcslen() prototype in string.h

[Andy Shevchenko]

On Tue, Mar 25, 2025 at 09:58:47AM -0700, Nathan Chancellor wrote:
> On Tue, Mar 25, 2025 at 06:17:34PM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 25, 2025 at 08:45:19AM -0700, Nathan Chancellor wrote:

...

> > > Rename the efi function to avoid the conflict.
> > 
> > Hmm... Why not split this to two, rename patch as a standalone makes sense to
> > me even outside of this series.
> 
> How so? If nls.h is not included in printk.c via string.h, which does
> not happen without this series, what value does the rename have? I do
> not mind splitting it up that way to keep things cleaner, I am just
> wondering what would be the justification in the changelog (I guess just
> that nls.h may get included in the future for some reason)?

Inside this series the justification is obvious (a.k.a. the same), outside
yes something like "Put EFI specific function to the respective namespace
to avoid potential clash in the future when including another header."

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RFC 2/2] wcslen() prototype in string.h

[Nathan Chancellor]

On Tue, Mar 25, 2025 at 07:05:28PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 25, 2025 at 09:58:47AM -0700, Nathan Chancellor wrote:
> > On Tue, Mar 25, 2025 at 06:17:34PM +0200, Andy Shevchenko wrote:
> > > On Tue, Mar 25, 2025 at 08:45:19AM -0700, Nathan Chancellor wrote:
> 
> ...
> 
> > > > Rename the efi function to avoid the conflict.
> > > 
> > > Hmm... Why not split this to two, rename patch as a standalone makes sense to
> > > me even outside of this series.
> > 
> > How so? If nls.h is not included in printk.c via string.h, which does
> > not happen without this series, what value does the rename have? I do
> > not mind splitting it up that way to keep things cleaner, I am just
> > wondering what would be the justification in the changelog (I guess just
> > that nls.h may get included in the future for some reason)?
> 
> Inside this series the justification is obvious (a.k.a. the same), outside
> yes something like "Put EFI specific function to the respective namespace
> to avoid potential clash in the future when including another header."

Okay, sounds reasonable to me. This is what I ended up with for that
change, which will become patch one of the series.

From f79267c05d2853c034e2c490f171aed064b85dd6 Mon Sep 17 00:00:00 2001
From: Nathan Chancellor <nathan@kernel.org>
Date: Tue, 25 Mar 2025 14:31:25 -0700
Subject: [PATCH] efi: libstub: Rename utf8_to_utf32() to avoid collision with
 NLS version

A future change will add nls.h to string.h, which will break the libstub
build because NLS includes its own version of utf8_to_utf32():

  drivers/firmware/efi/libstub/printk.c:27:5: error: static declaration of 'utf8_to_utf32' follows non-static declaration
     27 | u32 utf8_to_utf32(const u8 **s8)
        |     ^
  include/linux/nls.h:55:12: note: previous declaration is here
     55 | extern int utf8_to_utf32(const u8 *s, int len, unicode_t *pu);
        |            ^
  drivers/firmware/efi/libstub/printk.c:85:26: error: too few arguments to function call, expected 3, have 1
     85 |                 c32 = utf8_to_utf32(&s8);
        |                       ~~~~~~~~~~~~~    ^
  include/linux/nls.h:55:12: note: 'utf8_to_utf32' declared here
     55 | extern int utf8_to_utf32(const u8 *s, int len, unicode_t *pu);
        |            ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  2 errors generated.

Rename the libstub version since it is static to avoid the conflict.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/firmware/efi/libstub/printk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/libstub/printk.c b/drivers/firmware/efi/libstub/printk.c
index 3a67a2cea7bd..334f7e89845c 100644
--- a/drivers/firmware/efi/libstub/printk.c
+++ b/drivers/firmware/efi/libstub/printk.c
@@ -24,7 +24,7 @@ void efi_char16_puts(efi_char16_t *str)
 }
 
 static
-u32 utf8_to_utf32(const u8 **s8)
+u32 efi_utf8_to_utf32(const u8 **s8)
 {
 	u32 c32;
 	u8 c0, cx;
@@ -82,7 +82,7 @@ void efi_puts(const char *str)
 	while (*s8) {
 		if (*s8 == '\n')
 			buf[pos++] = L'\r';
-		c32 = utf8_to_utf32(&s8);
+		c32 = efi_utf8_to_utf32(&s8);
 		if (c32 < 0x10000) {
 			/* Characters in plane 0 use a single word. */
 			buf[pos++] = c32;
-- 
2.49.0

Then the first patch (which becomes the second) becomes:

diff --git a/include/linux/string.h b/include/linux/string.h
index 0403a4ca4c11..45e01cf3434c 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -7,6 +7,7 @@
 #include <linux/cleanup.h>	/* for DEFINE_FREE() */
 #include <linux/compiler.h>	/* for inline */
 #include <linux/types.h>	/* for size_t */
+#include <linux/nls.h>		/* for wchar_t */
 #include <linux/stddef.h>	/* for NULL */
 #include <linux/err.h>		/* for ERR_PTR() */
 #include <linux/errno.h>	/* for E2BIG */
@@ -203,6 +204,7 @@ extern __kernel_size_t strlen(const char *);
 #ifndef __HAVE_ARCH_STRNLEN
 extern __kernel_size_t strnlen(const char *,__kernel_size_t);
 #endif
+extern __kernel_size_t wcslen(const wchar_t *s);
 #ifndef __HAVE_ARCH_STRPBRK
 extern char * strpbrk(const char *,const char *);
 #endif
diff --git a/lib/string.c b/lib/string.c
index eb4486ed40d2..1aa09925254b 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -21,6 +21,7 @@
 #include <linux/errno.h>
 #include <linux/limits.h>
 #include <linux/linkage.h>
+#include <linux/nls.h>
 #include <linux/stddef.h>
 #include <linux/string.h>
 #include <linux/types.h>
@@ -429,6 +430,16 @@ size_t strnlen(const char *s, size_t count)
 EXPORT_SYMBOL(strnlen);
 #endif
 
+size_t wcslen(const wchar_t *s)
+{
+	const wchar_t *sc;
+
+	for (sc = s; *sc != '\0'; ++sc)
+		/* nothing */;
+	return sc - s;
+}
+EXPORT_SYMBOL(wcslen);
+
 #ifndef __HAVE_ARCH_STRSPN
 /**
  * strspn - Calculate the length of the initial substring of @s which only contain letters in @accept
---

with basically the same changelog.

I will send v2 either tomorrow afternoon or Thursday morning to give a
little more time for initial review/comments.

Thanks for the quick feedback!

Cheers,
Nathan

Re: [PATCH RFC 2/2] wcslen() prototype in string.h

[Nathan Chancellor]

On Tue, Mar 25, 2025 at 02:45:21PM -0700, Nathan Chancellor wrote:
> Okay, sounds reasonable to me. This is what I ended up with for that
> change, which will become patch one of the series.
...
> diff --git a/drivers/firmware/efi/libstub/printk.c b/drivers/firmware/efi/libstub/printk.c
> index 3a67a2cea7bd..334f7e89845c 100644
> --- a/drivers/firmware/efi/libstub/printk.c
> +++ b/drivers/firmware/efi/libstub/printk.c
> @@ -24,7 +24,7 @@ void efi_char16_puts(efi_char16_t *str)
>  }
>  
>  static
> -u32 utf8_to_utf32(const u8 **s8)
> +u32 efi_utf8_to_utf32(const u8 **s8)
>  {
>  	u32 c32;
>  	u8 c0, cx;
> @@ -82,7 +82,7 @@ void efi_puts(const char *str)
>  	while (*s8) {
>  		if (*s8 == '\n')
>  			buf[pos++] = L'\r';
> -		c32 = utf8_to_utf32(&s8);
> +		c32 = efi_utf8_to_utf32(&s8);
>  		if (c32 < 0x10000) {
>  			/* Characters in plane 0 use a single word. */
>  			buf[pos++] = c32;
> -- 
> 2.49.0
> 
> Then the first patch (which becomes the second) becomes:
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 0403a4ca4c11..45e01cf3434c 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -7,6 +7,7 @@
>  #include <linux/cleanup.h>	/* for DEFINE_FREE() */
>  #include <linux/compiler.h>	/* for inline */
>  #include <linux/types.h>	/* for size_t */
> +#include <linux/nls.h>		/* for wchar_t */

Good thing I waited :) This include makes s390 unhappy:

https://lore.kernel.org/202503260611.MDurOUhF-lkp@intel.com/

It is possible that should be fixed by adding -Wno-pointer-sign to
KBUILD_CFLAGS_DECOMPRESSOR so that arch/s390/boot matches the rest of
the kernel but...

> diff --git a/lib/string.c b/lib/string.c
> index eb4486ed40d2..1aa09925254b 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -21,6 +21,7 @@
>  #include <linux/errno.h>
>  #include <linux/limits.h>
>  #include <linux/linkage.h>
> +#include <linux/nls.h>
>  #include <linux/stddef.h>
>  #include <linux/string.h>
>  #include <linux/types.h>

I wonder if would be better to do something like the below patch in lieu
of the EFI change above (since there is no chance for a collision) then
change both of the includes for wchar_t in this diff to nls_types.h? I
have no strong opinion but this seems like it would be cleaner for the
sake of backports while not being a bad solution upstream?

Subject: [PATCH] include: Move typedefs in nls.h to their own header

In order to allow commonly included headers such as string.h to access
typedefs such as wchar_t without running into issues with the rest of
the NLS library, refactor the typedefs out into their own header that
can be included in a much safer manner.

Cc: stable@vger.kernel.org
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 include/linux/nls.h       | 19 +------------------
 include/linux/nls_types.h | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 18 deletions(-)
 create mode 100644 include/linux/nls_types.h

diff --git a/include/linux/nls.h b/include/linux/nls.h
index e0bf8367b274..3d416d1f60b6 100644
--- a/include/linux/nls.h
+++ b/include/linux/nls.h
@@ -3,24 +3,7 @@
 #define _LINUX_NLS_H
 
 #include <linux/init.h>
-
-/* Unicode has changed over the years.  Unicode code points no longer
- * fit into 16 bits; as of Unicode 5 valid code points range from 0
- * to 0x10ffff (17 planes, where each plane holds 65536 code points).
- *
- * The original decision to represent Unicode characters as 16-bit
- * wchar_t values is now outdated.  But plane 0 still includes the
- * most commonly used characters, so we will retain it.  The newer
- * 32-bit unicode_t type can be used when it is necessary to
- * represent the full Unicode character set.
- */
-
-/* Plane-0 Unicode character */
-typedef u16 wchar_t;
-#define MAX_WCHAR_T	0xffff
-
-/* Arbitrary Unicode character */
-typedef u32 unicode_t;
+#include <linux/nls_types.h>
 
 struct nls_table {
 	const char *charset;
diff --git a/include/linux/nls_types.h b/include/linux/nls_types.h
new file mode 100644
index 000000000000..8caefdba19b1
--- /dev/null
+++ b/include/linux/nls_types.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_NLS_TYPES_H
+#define _LINUX_NLS_TYPES_H
+
+#include <linux/types.h>
+
+/* Unicode has changed over the years.  Unicode code points no longer
+ * fit into 16 bits; as of Unicode 5 valid code points range from 0
+ * to 0x10ffff (17 planes, where each plane holds 65536 code points).
+ *
+ * The original decision to represent Unicode characters as 16-bit
+ * wchar_t values is now outdated.  But plane 0 still includes the
+ * most commonly used characters, so we will retain it.  The newer
+ * 32-bit unicode_t type can be used when it is necessary to
+ * represent the full Unicode character set.
+ */
+
+/* Plane-0 Unicode character */
+typedef u16 wchar_t;
+#define MAX_WCHAR_T	0xffff
+
+/* Arbitrary Unicode character */
+typedef u32 unicode_t;
+
+#endif /* _LINUX_NLS_TYPES_H */
-- 
2.49.0

diff --git a/include/linux/string.h b/include/linux/string.h
index 45e01cf3434c..4a48f8eac301 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -7,7 +7,7 @@
 #include <linux/cleanup.h>	/* for DEFINE_FREE() */
 #include <linux/compiler.h>	/* for inline */
 #include <linux/types.h>	/* for size_t */
-#include <linux/nls.h>		/* for wchar_t */
+#include <linux/nls_types.h>	/* for wchar_t */
 #include <linux/stddef.h>	/* for NULL */
 #include <linux/err.h>		/* for ERR_PTR() */
 #include <linux/errno.h>	/* for E2BIG */
diff --git a/lib/string.c b/lib/string.c
index 1aa09925254b..2c6f8c8f4159 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -21,7 +21,7 @@
 #include <linux/errno.h>
 #include <linux/limits.h>
 #include <linux/linkage.h>
-#include <linux/nls.h>
+#include <linux/nls_types.h>
 #include <linux/stddef.h>
 #include <linux/string.h>
 #include <linux/types.h>

Re: [PATCH RFC 2/2] wcslen() prototype in string.h

[Andy Shevchenko]

On Tue, Mar 25, 2025 at 05:33:03PM -0700, Nathan Chancellor wrote:
> On Tue, Mar 25, 2025 at 02:45:21PM -0700, Nathan Chancellor wrote:

...

> > +#include <linux/nls.h>		/* for wchar_t */
> 
> Good thing I waited :) This include makes s390 unhappy:
> 
> https://lore.kernel.org/202503260611.MDurOUhF-lkp@intel.com/
> 
> It is possible that should be fixed by adding -Wno-pointer-sign to
> KBUILD_CFLAGS_DECOMPRESSOR so that arch/s390/boot matches the rest of
> the kernel but...

Ah, yes, you beat me up to commenting on this, the string.h and string.c made
in a way that they may be and are used in early boot code, i.e. it must not be
dirtyfied with the kernel code.

...

> >  #include <linux/errno.h>
> >  #include <linux/limits.h>
> >  #include <linux/linkage.h>
> > +#include <linux/nls.h>
> >  #include <linux/stddef.h>
> >  #include <linux/string.h>
> >  #include <linux/types.h>
> 
> I wonder if would be better to do something like the below patch in lieu
> of the EFI change above (since there is no chance for a collision) then
> change both of the includes for wchar_t in this diff to nls_types.h? I
> have no strong opinion but this seems like it would be cleaner for the
> sake of backports while not being a bad solution upstream?

>  #define _LINUX_NLS_H
>  
>  #include <linux/init.h>

As I just replied to your previous mail, consider fixing this list as well
by adding module.h and types.h.

...

Overall, can you browse the Ingo's series [1] for the stuff related to this,
if any?

I would avoid doing double efforts or different approaches if we already have
something ready.

[1]: https://lore.kernel.org/linux-kernel/YjBr10JXLGHfEFfi@gmail.com/

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RFC 2/2] wcslen() prototype in string.h

[Nathan Chancellor]

On Wed, Mar 26, 2025 at 10:59:52AM +0200, Andy Shevchenko wrote:
> On Tue, Mar 25, 2025 at 05:33:03PM -0700, Nathan Chancellor wrote:
> >  #define _LINUX_NLS_H
> >  
> >  #include <linux/init.h>
> 
> As I just replied to your previous mail, consider fixing this list as well
> by adding module.h and types.h.
> 
> ...
> 
> Overall, can you browse the Ingo's series [1] for the stuff related to this,
> if any?
> 
> I would avoid doing double efforts or different approaches if we already have
> something ready.

In Ingo's last fast-headers tree [1], nls.h only has export.h and init.h
included, so it does not look like anything around this was changed from
what I can tell.

types.h is going to be included via the new nls_types.h and while it
does definitely look like module.h should be included, I do not really
have the time and build capacity at the moment to incorporate testing
that change into this series. I will stick with these two changes for
now then I, you, or someone else can revisit cleaning up nls.h later.

[1]: https://web.git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git/tree/include/linux/nls.h?h=sched/headers

Cheers,
Nathan

Re: [PATCH RFC 2/2] wcslen() prototype in string.h

[Andy Shevchenko]

On Wed, Mar 26, 2025 at 08:37:56AM -0700, Nathan Chancellor wrote:
> On Wed, Mar 26, 2025 at 10:59:52AM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 25, 2025 at 05:33:03PM -0700, Nathan Chancellor wrote:

...

> > >  #include <linux/init.h>
> > 
> > As I just replied to your previous mail, consider fixing this list as well
> > by adding module.h and types.h.

...

> > Overall, can you browse the Ingo's series [1] for the stuff related to this,
> > if any?
> > 
> > I would avoid doing double efforts or different approaches if we already have
> > something ready.
> 
> In Ingo's last fast-headers tree [1], nls.h only has export.h and init.h
> included, so it does not look like anything around this was changed from
> what I can tell.

Thanks for checking!

> types.h is going to be included via the new nls_types.h and while it
> does definitely look like module.h should be included, I do not really
> have the time and build capacity at the moment to incorporate testing
> that change into this series. I will stick with these two changes for
> now then I, you, or someone else can revisit cleaning up nls.h later.

Fair enough.

> [1]: https://web.git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git/tree/include/linux/nls.h?h=sched/headers

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RFC 2/2] wcslen() prototype in string.h

[Andy Shevchenko]

On Tue, Mar 25, 2025 at 02:45:16PM -0700, Nathan Chancellor wrote:
> On Tue, Mar 25, 2025 at 07:05:28PM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 25, 2025 at 09:58:47AM -0700, Nathan Chancellor wrote:
> > > On Tue, Mar 25, 2025 at 06:17:34PM +0200, Andy Shevchenko wrote:
> > > > On Tue, Mar 25, 2025 at 08:45:19AM -0700, Nathan Chancellor wrote:

...

> > > > > Rename the efi function to avoid the conflict.
> > > > 
> > > > Hmm... Why not split this to two, rename patch as a standalone makes sense to
> > > > me even outside of this series.
> > > 
> > > How so? If nls.h is not included in printk.c via string.h, which does
> > > not happen without this series, what value does the rename have? I do
> > > not mind splitting it up that way to keep things cleaner, I am just
> > > wondering what would be the justification in the changelog (I guess just
> > > that nls.h may get included in the future for some reason)?
> > 
> > Inside this series the justification is obvious (a.k.a. the same), outside
> > yes something like "Put EFI specific function to the respective namespace
> > to avoid potential clash in the future when including another header."
> 
> Okay, sounds reasonable to me. This is what I ended up with for that
> change, which will become patch one of the series.

LGTM.
However, now I'm thinking about nls.h appearing in the string.h.
The former actually lacks two headers in it: module.h and types.h.
I'm wondering if fixing nls.h would be even possible after your series.


> with basically the same changelog.
> 
> I will send v2 either tomorrow afternoon or Thursday morning to give a
> little more time for initial review/comments.

-- 
With Best Regards,
Andy Shevchenko