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 <nat...@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 <nat...@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
