dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/xe: replace basename() with portable strrchr()
@ 2025-08-20 20:16 Carlos Llamas
  2025-08-20 21:15 ` Lucas De Marchi
  2025-08-20 21:18 ` [PATCH] drm/xe: replace basename() with portable strrchr() Tiffany Yang
  0 siblings, 2 replies; 11+ messages in thread
From: Carlos Llamas @ 2025-08-20 20:16 UTC (permalink / raw)
  To: Lucas De Marchi, Thomas Hellström, Rodrigo Vivi,
	David Airlie, Simona Vetter, Matt Atwood
  Cc: kernel-team, linux-kernel, Carlos Llamas,
	open list:INTEL DRM XE DRIVER (Lunar Lake and newer),
	open list:DRM DRIVERS

Commit b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
introduced a call to basename(). The GNU version of this function is not
portable and fails to build with alternative libc implementations like
musl or bionic. This causes the following build error:

  drivers/gpu/drm/xe/xe_gen_wa_oob.c:130:12: error: assignment to ‘const char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
    130 |         fn = basename(fn);
        |            ^

While a POSIX version of basename() could be used, it would require a
separate header plus the behavior differs from GNU version in that it
might modify its argument. Not great.

Instead replace basename() with a strrchr() based implementation which
provides the same functionality and avoid portability issues.

Fixes: b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 drivers/gpu/drm/xe/xe_gen_wa_oob.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_gen_wa_oob.c b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
index 6581cb0f0e59..0a94a045bcea 100644
--- a/drivers/gpu/drm/xe/xe_gen_wa_oob.c
+++ b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
@@ -125,9 +125,11 @@ static int parse(FILE *input, FILE *csource, FILE *cheader, char *prefix)
 
 static int fn_to_prefix(const char *fn, char *prefix, size_t size)
 {
+	const char *base;
 	size_t len;
 
-	fn = basename(fn);
+	base = strrchr(fn, '/');
+	fn = base ? base + 1 : fn;
 	len = strlen(fn);
 
 	if (len > size - 1)
-- 
2.51.0.rc1.193.gad69d77794-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/xe: replace basename() with portable strrchr()
  2025-08-20 20:16 [PATCH] drm/xe: replace basename() with portable strrchr() Carlos Llamas
@ 2025-08-20 21:15 ` Lucas De Marchi
  2025-08-20 23:05   ` Carlos Llamas
  2025-08-20 21:18 ` [PATCH] drm/xe: replace basename() with portable strrchr() Tiffany Yang
  1 sibling, 1 reply; 11+ messages in thread
From: Lucas De Marchi @ 2025-08-20 21:15 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
	Matt Atwood, kernel-team, linux-kernel,
	open list:INTEL DRM XE DRIVER (Lunar Lake and newer),
	open list:DRM DRIVERS

On Wed, Aug 20, 2025 at 08:16:11PM +0000, Carlos Llamas wrote:
>Commit b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
>introduced a call to basename(). The GNU version of this function is not
>portable and fails to build with alternative libc implementations like
>musl or bionic. This causes the following build error:
>
>  drivers/gpu/drm/xe/xe_gen_wa_oob.c:130:12: error: assignment to ‘const char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
>    130 |         fn = basename(fn);
>        |            ^
>
>While a POSIX version of basename() could be used, it would require a
>separate header plus the behavior differs from GNU version in that it
>might modify its argument. Not great.
>
>Instead replace basename() with a strrchr() based implementation which
>provides the same functionality and avoid portability issues.
>
>Fixes: b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
>Signed-off-by: Carlos Llamas <cmllamas@google.com>
>---
> drivers/gpu/drm/xe/xe_gen_wa_oob.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_gen_wa_oob.c b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>index 6581cb0f0e59..0a94a045bcea 100644
>--- a/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>+++ b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>@@ -125,9 +125,11 @@ static int parse(FILE *input, FILE *csource, FILE *cheader, char *prefix)
>
> static int fn_to_prefix(const char *fn, char *prefix, size_t size)
> {
>+	const char *base;
> 	size_t len;
>
>-	fn = basename(fn);
>+	base = strrchr(fn, '/');
>+	fn = base ? base + 1 : fn;

I think just a xbasename() helper like we've added in kmod would be
preferred:
https://github.com/kmod-project/kmod/commit/11eb9bc67c319900ab00523997323a97d2d08ad2

Alternativelly add it somewhere that can be shared across the userspace
tools in the kernel tree to fix the mess that we have here:

	git grep basename -- tools/**.c

Some dup the arg simply to be able to use the libgen.h version, some use
one or the other on purpose, etc etc.

Lucas De Marchi

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/xe: replace basename() with portable strrchr()
  2025-08-20 20:16 [PATCH] drm/xe: replace basename() with portable strrchr() Carlos Llamas
  2025-08-20 21:15 ` Lucas De Marchi
@ 2025-08-20 21:18 ` Tiffany Yang
  1 sibling, 0 replies; 11+ messages in thread
From: Tiffany Yang @ 2025-08-20 21:18 UTC (permalink / raw)
  To: 'Carlos Llamas' via kernel-team
  Cc: Lucas De Marchi, Thomas Hellström, Rodrigo Vivi,
	David Airlie, Simona Vetter, Matt Atwood, Carlos Llamas,
	linux-kernel,
	open list:INTEL DRM XE DRIVER (Lunar Lake and newer),
	open list:DRM DRIVERS

"'Carlos Llamas' via kernel-team" <kernel-team@android.com> writes:

> Commit b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
> introduced a call to basename(). The GNU version of this function is not
> portable and fails to build with alternative libc implementations like
> musl or bionic. This causes the following build error:

>    drivers/gpu/drm/xe/xe_gen_wa_oob.c:130:12: error: assignment to ‘const  
> char *’ from ‘int’ makes pointer from integer without a cast  
> [-Wint-conversion]
>      130 |         fn = basename(fn);
>          |            ^

> While a POSIX version of basename() could be used, it would require a
> separate header plus the behavior differs from GNU version in that it
> might modify its argument. Not great.

> Instead replace basename() with a strrchr() based implementation which
> provides the same functionality and avoid portability issues.

Nit: s/avoid/avoids. Other than that,


> Fixes: b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> ---
>   drivers/gpu/drm/xe/xe_gen_wa_oob.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)


Reviewed-by: Tiffany Yang <ynaffit@google.com>

-- 
Tiffany Y. Yang

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/xe: replace basename() with portable strrchr()
  2025-08-20 21:15 ` Lucas De Marchi
@ 2025-08-20 23:05   ` Carlos Llamas
  2025-08-21 21:00     ` Lucas De Marchi
  0 siblings, 1 reply; 11+ messages in thread
From: Carlos Llamas @ 2025-08-20 23:05 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
	Matt Atwood, kernel-team, linux-kernel,
	open list:INTEL DRM XE DRIVER (Lunar Lake and newer),
	open list:DRM DRIVERS

On Wed, Aug 20, 2025 at 04:15:47PM -0500, Lucas De Marchi wrote:
> On Wed, Aug 20, 2025 at 08:16:11PM +0000, Carlos Llamas wrote:
> > Commit b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
> > introduced a call to basename(). The GNU version of this function is not
> > portable and fails to build with alternative libc implementations like
> > musl or bionic. This causes the following build error:
> > 
> >  drivers/gpu/drm/xe/xe_gen_wa_oob.c:130:12: error: assignment to ‘const char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
> >    130 |         fn = basename(fn);
> >        |            ^
> > 
> > While a POSIX version of basename() could be used, it would require a
> > separate header plus the behavior differs from GNU version in that it
> > might modify its argument. Not great.
> > 
> > Instead replace basename() with a strrchr() based implementation which
> > provides the same functionality and avoid portability issues.
> > 
> > Fixes: b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
> > Signed-off-by: Carlos Llamas <cmllamas@google.com>
> > ---
> > drivers/gpu/drm/xe/xe_gen_wa_oob.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_gen_wa_oob.c b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
> > index 6581cb0f0e59..0a94a045bcea 100644
> > --- a/drivers/gpu/drm/xe/xe_gen_wa_oob.c
> > +++ b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
> > @@ -125,9 +125,11 @@ static int parse(FILE *input, FILE *csource, FILE *cheader, char *prefix)
> > 
> > static int fn_to_prefix(const char *fn, char *prefix, size_t size)
> > {
> > +	const char *base;
> > 	size_t len;
> > 
> > -	fn = basename(fn);
> > +	base = strrchr(fn, '/');
> > +	fn = base ? base + 1 : fn;
> 
> I think just a xbasename() helper like we've added in kmod would be
> preferred:
> https://github.com/kmod-project/kmod/commit/11eb9bc67c319900ab00523997323a97d2d08ad2
> 
> Alternativelly add it somewhere that can be shared across the userspace
> tools in the kernel tree to fix the mess that we have here:
> 
> 	git grep basename -- tools/**.c
> 

This sounds like a nice idea. However, there is no "centralized" shared
includes/ across the userspace tools that I'm aware of?

> Some dup the arg simply to be able to use the libgen.h version, some use
> one or the other on purpose, etc etc.
> 

Yeah, and I can force the POSIX version if you prefer. I just personally
think the strrchr() alternative ends up being simpler.

--
Carlos Llamas

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/xe: replace basename() with portable strrchr()
  2025-08-20 23:05   ` Carlos Llamas
@ 2025-08-21 21:00     ` Lucas De Marchi
  2025-08-21 21:32       ` Carlos Llamas
  2025-08-21 22:00       ` [PATCH v2] drm/xe: switch to local __basename() helper Carlos Llamas
  0 siblings, 2 replies; 11+ messages in thread
From: Lucas De Marchi @ 2025-08-21 21:00 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
	Matt Atwood, kernel-team, linux-kernel,
	open list:INTEL DRM XE DRIVER (Lunar Lake and newer),
	open list:DRM DRIVERS

On Wed, Aug 20, 2025 at 11:05:47PM +0000, Carlos Llamas wrote:
>On Wed, Aug 20, 2025 at 04:15:47PM -0500, Lucas De Marchi wrote:
>> On Wed, Aug 20, 2025 at 08:16:11PM +0000, Carlos Llamas wrote:
>> > Commit b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
>> > introduced a call to basename(). The GNU version of this function is not
>> > portable and fails to build with alternative libc implementations like
>> > musl or bionic. This causes the following build error:
>> >
>> >  drivers/gpu/drm/xe/xe_gen_wa_oob.c:130:12: error: assignment to ‘const char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
>> >    130 |         fn = basename(fn);
>> >        |            ^
>> >
>> > While a POSIX version of basename() could be used, it would require a
>> > separate header plus the behavior differs from GNU version in that it
>> > might modify its argument. Not great.
>> >
>> > Instead replace basename() with a strrchr() based implementation which
>> > provides the same functionality and avoid portability issues.
>> >
>> > Fixes: b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
>> > Signed-off-by: Carlos Llamas <cmllamas@google.com>
>> > ---
>> > drivers/gpu/drm/xe/xe_gen_wa_oob.c | 4 +++-
>> > 1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/xe/xe_gen_wa_oob.c b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>> > index 6581cb0f0e59..0a94a045bcea 100644
>> > --- a/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>> > +++ b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>> > @@ -125,9 +125,11 @@ static int parse(FILE *input, FILE *csource, FILE *cheader, char *prefix)
>> >
>> > static int fn_to_prefix(const char *fn, char *prefix, size_t size)
>> > {
>> > +	const char *base;
>> > 	size_t len;
>> >
>> > -	fn = basename(fn);
>> > +	base = strrchr(fn, '/');
>> > +	fn = base ? base + 1 : fn;
>>
>> I think just a xbasename() helper like we've added in kmod would be
>> preferred:
>> https://github.com/kmod-project/kmod/commit/11eb9bc67c319900ab00523997323a97d2d08ad2
>>
>> Alternativelly add it somewhere that can be shared across the userspace
>> tools in the kernel tree to fix the mess that we have here:
>>
>> 	git grep basename -- tools/**.c
>>
>
>This sounds like a nice idea. However, there is no "centralized" shared
>includes/ across the userspace tools that I'm aware of?
>
>> Some dup the arg simply to be able to use the libgen.h version, some use
>> one or the other on purpose, etc etc.
>>
>
>Yeah, and I can force the POSIX version if you prefer. I just personally
>think the strrchr() alternative ends up being simpler.

IMO the POSIX version is horrible. Let's add a xbasename() in this
xe_gen_wa_oob.c and use it:

/*
  * Avoid the libgen.h vs string.h differences or lack thereof, just use
  * our own.
  */
static const char *xbasename(const char *s)
{
	const char *p = strrchr(s, '/');
	return p ? p + 1 : s;
}

static int fn_to_prefix(const char *fn, char *prefix, size_t size)
{
...
	fn = xbasename(fn);
...
}

Lucas De Marchi

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/xe: replace basename() with portable strrchr()
  2025-08-21 21:00     ` Lucas De Marchi
@ 2025-08-21 21:32       ` Carlos Llamas
  2025-08-21 22:00       ` [PATCH v2] drm/xe: switch to local __basename() helper Carlos Llamas
  1 sibling, 0 replies; 11+ messages in thread
From: Carlos Llamas @ 2025-08-21 21:32 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
	Matt Atwood, kernel-team, linux-kernel,
	open list:INTEL DRM XE DRIVER (Lunar Lake and newer),
	open list:DRM DRIVERS

On Thu, Aug 21, 2025 at 04:00:33PM -0500, Lucas De Marchi wrote:
> 
> IMO the POSIX version is horrible. Let's add a xbasename() in this
> xe_gen_wa_oob.c and use it:
> 
> /*
>  * Avoid the libgen.h vs string.h differences or lack thereof, just use
>  * our own.
>  */
> static const char *xbasename(const char *s)
> {
> 	const char *p = strrchr(s, '/');
> 	return p ? p + 1 : s;
> }
> 
> static int fn_to_prefix(const char *fn, char *prefix, size_t size)
> {
> ...
> 	fn = xbasename(fn);
> ...
> }
> 

Sounds good. Would you mind if I use __basename()? I'll send the v2 in a
bit.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] drm/xe: switch to local __basename() helper
  2025-08-21 21:00     ` Lucas De Marchi
  2025-08-21 21:32       ` Carlos Llamas
@ 2025-08-21 22:00       ` Carlos Llamas
  2025-08-23 11:56         ` Lucas De Marchi
  1 sibling, 1 reply; 11+ messages in thread
From: Carlos Llamas @ 2025-08-21 22:00 UTC (permalink / raw)
  To: Lucas De Marchi, Thomas Hellström, Rodrigo Vivi,
	David Airlie, Simona Vetter, Matt Atwood
  Cc: kernel-team, linux-kernel, Carlos Llamas, Tiffany Yang,
	open list:INTEL DRM XE DRIVER (Lunar Lake and newer),
	open list:DRM DRIVERS

Commit b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
introduced a call to basename(). The GNU version of this function is not
portable and fails to build with alternative libc implementations like
musl or bionic. This causes the following build error:

  drivers/gpu/drm/xe/xe_gen_wa_oob.c:130:12: error: assignment to ‘const char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
    130 |         fn = basename(fn);
        |            ^

While a POSIX version of basename() could be used, it would require a
separate header plus the behavior differs from GNU version in that it
might modify its argument. Not great.

Instead, implement a local __basename() helper based on strrchr() that
provides the same functionality and avoids portability issues.

Fixes: b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Tiffany Yang <ynaffit@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
v2:
 - Wrap changes in a helper function per Luca's feedback.
 - Fix typo in commit log: s/avoid/avoids/ per Tiffany.
 - Update commit log.
 - Collect tags.

v1:
https://lore.kernel.org/all/20250820201612.2549797-1-cmllamas@google.com/

 drivers/gpu/drm/xe/xe_gen_wa_oob.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_gen_wa_oob.c b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
index 6581cb0f0e59..c18faccdeb90 100644
--- a/drivers/gpu/drm/xe/xe_gen_wa_oob.c
+++ b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
@@ -123,11 +123,19 @@ static int parse(FILE *input, FILE *csource, FILE *cheader, char *prefix)
 	return 0;
 }
 
+/* Avoid GNU vs POSIX basename() discrepancy, just use our own */
+static const char *__basename(const char *s)
+{
+	const char *p = strrchr(s, '/');
+
+	return p ? p + 1 : s;
+}
+
 static int fn_to_prefix(const char *fn, char *prefix, size_t size)
 {
 	size_t len;
 
-	fn = basename(fn);
+	fn = __basename(fn);
 	len = strlen(fn);
 
 	if (len > size - 1)
-- 
2.51.0.rc2.233.g662b1ed5c5-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] drm/xe: switch to local __basename() helper
  2025-08-21 22:00       ` [PATCH v2] drm/xe: switch to local __basename() helper Carlos Llamas
@ 2025-08-23 11:56         ` Lucas De Marchi
  2025-08-25 15:49           ` Carlos Llamas
  2025-08-25 15:57           ` [PATCH v3] drm/xe: switch to local xbasename() helper Carlos Llamas
  0 siblings, 2 replies; 11+ messages in thread
From: Lucas De Marchi @ 2025-08-23 11:56 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
	Matt Atwood, kernel-team, linux-kernel, Tiffany Yang,
	open list:INTEL DRM XE DRIVER (Lunar Lake and newer),
	open list:DRM DRIVERS

On Thu, Aug 21, 2025 at 10:00:53PM +0000, Carlos Llamas wrote:
>Commit b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
>introduced a call to basename(). The GNU version of this function is not
>portable and fails to build with alternative libc implementations like
>musl or bionic. This causes the following build error:
>
>  drivers/gpu/drm/xe/xe_gen_wa_oob.c:130:12: error: assignment to ‘const char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
>    130 |         fn = basename(fn);
>        |            ^
>
>While a POSIX version of basename() could be used, it would require a
>separate header plus the behavior differs from GNU version in that it
>might modify its argument. Not great.
>
>Instead, implement a local __basename() helper based on strrchr() that

double underscore is reserved for libc in userspace
(https://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html):

	(...) all identifiers regardless of use that begin with either two
	underscores or an underscore followed by a capital letter are reserved
	names. This is so that the library and header files can define
	functions, variables, and macros for internal purposes without risk of
	conflict with names in user programs.

>provides the same functionality and avoids portability issues.

Unfortunately with that name it could created other portability issues.

Lucas De Marchi

>
>Fixes: b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
>Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
>Reviewed-by: Tiffany Yang <ynaffit@google.com>
>Signed-off-by: Carlos Llamas <cmllamas@google.com>
>---
>v2:
> - Wrap changes in a helper function per Luca's feedback.
> - Fix typo in commit log: s/avoid/avoids/ per Tiffany.
> - Update commit log.
> - Collect tags.
>
>v1:
>https://lore.kernel.org/all/20250820201612.2549797-1-cmllamas@google.com/
>
> drivers/gpu/drm/xe/xe_gen_wa_oob.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_gen_wa_oob.c b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>index 6581cb0f0e59..c18faccdeb90 100644
>--- a/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>+++ b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>@@ -123,11 +123,19 @@ static int parse(FILE *input, FILE *csource, FILE *cheader, char *prefix)
> 	return 0;
> }
>
>+/* Avoid GNU vs POSIX basename() discrepancy, just use our own */
>+static const char *__basename(const char *s)
>+{
>+	const char *p = strrchr(s, '/');
>+
>+	return p ? p + 1 : s;
>+}
>+
> static int fn_to_prefix(const char *fn, char *prefix, size_t size)
> {
> 	size_t len;
>
>-	fn = basename(fn);
>+	fn = __basename(fn);
> 	len = strlen(fn);
>
> 	if (len > size - 1)
>-- 
>2.51.0.rc2.233.g662b1ed5c5-goog
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] drm/xe: switch to local __basename() helper
  2025-08-23 11:56         ` Lucas De Marchi
@ 2025-08-25 15:49           ` Carlos Llamas
  2025-08-25 15:57           ` [PATCH v3] drm/xe: switch to local xbasename() helper Carlos Llamas
  1 sibling, 0 replies; 11+ messages in thread
From: Carlos Llamas @ 2025-08-25 15:49 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
	Matt Atwood, kernel-team, linux-kernel, Tiffany Yang,
	open list:INTEL DRM XE DRIVER (Lunar Lake and newer),
	open list:DRM DRIVERS

On Sat, Aug 23, 2025 at 06:56:30AM -0500, Lucas De Marchi wrote:
> On Thu, Aug 21, 2025 at 10:00:53PM +0000, Carlos Llamas wrote:
> > Commit b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
> > introduced a call to basename(). The GNU version of this function is not
> > portable and fails to build with alternative libc implementations like
> > musl or bionic. This causes the following build error:
> > 
> >  drivers/gpu/drm/xe/xe_gen_wa_oob.c:130:12: error: assignment to ‘const char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
> >    130 |         fn = basename(fn);
> >        |            ^
> > 
> > While a POSIX version of basename() could be used, it would require a
> > separate header plus the behavior differs from GNU version in that it
> > might modify its argument. Not great.
> > 
> > Instead, implement a local __basename() helper based on strrchr() that
> 
> double underscore is reserved for libc in userspace
> (https://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html):
> 
> 	(...) all identifiers regardless of use that begin with either two
> 	underscores or an underscore followed by a capital letter are reserved
> 	names. This is so that the library and header files can define
> 	functions, variables, and macros for internal purposes without risk of
> 	conflict with names in user programs.
> 

I see, xbasename() it is then...

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3] drm/xe: switch to local xbasename() helper
  2025-08-23 11:56         ` Lucas De Marchi
  2025-08-25 15:49           ` Carlos Llamas
@ 2025-08-25 15:57           ` Carlos Llamas
  2025-08-25 16:01             ` Lucas De Marchi
  1 sibling, 1 reply; 11+ messages in thread
From: Carlos Llamas @ 2025-08-25 15:57 UTC (permalink / raw)
  To: Lucas De Marchi, Thomas Hellström, Rodrigo Vivi,
	David Airlie, Simona Vetter, Matt Atwood
  Cc: kernel-team, linux-kernel, Carlos Llamas, Tiffany Yang,
	open list:INTEL DRM XE DRIVER (Lunar Lake and newer),
	open list:DRM DRIVERS

Commit b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
introduced a call to basename(). The GNU version of this function is not
portable and fails to build with alternative libc implementations like
musl or bionic. This causes the following build error:

  drivers/gpu/drm/xe/xe_gen_wa_oob.c:130:12: error: assignment to ‘const char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
    130 |         fn = basename(fn);
        |            ^

While a POSIX version of basename() could be used, it would require a
separate header plus the behavior differs from GNU version in that it
might modify its argument. Not great.

Instead, implement a local xbasename() helper based on strrchr() that
provides the same functionality and avoids portability issues.

Fixes: b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Tiffany Yang <ynaffit@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
v3:
 - Switch __basename() -> xbasename() per Lucas.

v2:
https://lore.kernel.org/all/20250821220054.3700783-1-cmllamas@google.com/
 - Wrap changes in a helper function per Lucas' feedback.
 - Fix typo in commit log: s/avoid/avoids/ per Tiffany.
 - Update commit log.
 - Collect tags.

v1:
https://lore.kernel.org/all/20250820201612.2549797-1-cmllamas@google.com/
 drivers/gpu/drm/xe/xe_gen_wa_oob.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_gen_wa_oob.c b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
index 6581cb0f0e59..247e41c1c48d 100644
--- a/drivers/gpu/drm/xe/xe_gen_wa_oob.c
+++ b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
@@ -123,11 +123,19 @@ static int parse(FILE *input, FILE *csource, FILE *cheader, char *prefix)
 	return 0;
 }
 
+/* Avoid GNU vs POSIX basename() discrepancy, just use our own */
+static const char *xbasename(const char *s)
+{
+	const char *p = strrchr(s, '/');
+
+	return p ? p + 1 : s;
+}
+
 static int fn_to_prefix(const char *fn, char *prefix, size_t size)
 {
 	size_t len;
 
-	fn = basename(fn);
+	fn = xbasename(fn);
 	len = strlen(fn);
 
 	if (len > size - 1)
-- 
2.51.0.rc2.233.g662b1ed5c5-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] drm/xe: switch to local xbasename() helper
  2025-08-25 15:57           ` [PATCH v3] drm/xe: switch to local xbasename() helper Carlos Llamas
@ 2025-08-25 16:01             ` Lucas De Marchi
  0 siblings, 0 replies; 11+ messages in thread
From: Lucas De Marchi @ 2025-08-25 16:01 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
	Matt Atwood, kernel-team, linux-kernel, Tiffany Yang,
	open list:INTEL DRM XE DRIVER (Lunar Lake and newer),
	open list:DRM DRIVERS

On Mon, Aug 25, 2025 at 03:57:42PM +0000, Carlos Llamas wrote:
>Commit b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
>introduced a call to basename(). The GNU version of this function is not
>portable and fails to build with alternative libc implementations like
>musl or bionic. This causes the following build error:
>
>  drivers/gpu/drm/xe/xe_gen_wa_oob.c:130:12: error: assignment to ‘const char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
>    130 |         fn = basename(fn);
>        |            ^
>
>While a POSIX version of basename() could be used, it would require a
>separate header plus the behavior differs from GNU version in that it
>might modify its argument. Not great.
>
>Instead, implement a local xbasename() helper based on strrchr() that
>provides the same functionality and avoids portability issues.
>
>Fixes: b0a2ee5567ab ("drm/xe: prepare xe_gen_wa_oob to be multi-use")
>Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
>Reviewed-by: Tiffany Yang <ynaffit@google.com>
>Signed-off-by: Carlos Llamas <cmllamas@google.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

I'll push to drm-xe-next later today. Thanks.

Lucas De Marchi

>---
>v3:
> - Switch __basename() -> xbasename() per Lucas.
>
>v2:
>https://lore.kernel.org/all/20250821220054.3700783-1-cmllamas@google.com/
> - Wrap changes in a helper function per Lucas' feedback.
> - Fix typo in commit log: s/avoid/avoids/ per Tiffany.
> - Update commit log.
> - Collect tags.
>
>v1:
>https://lore.kernel.org/all/20250820201612.2549797-1-cmllamas@google.com/
> drivers/gpu/drm/xe/xe_gen_wa_oob.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_gen_wa_oob.c b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>index 6581cb0f0e59..247e41c1c48d 100644
>--- a/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>+++ b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>@@ -123,11 +123,19 @@ static int parse(FILE *input, FILE *csource, FILE *cheader, char *prefix)
> 	return 0;
> }
>
>+/* Avoid GNU vs POSIX basename() discrepancy, just use our own */
>+static const char *xbasename(const char *s)
>+{
>+	const char *p = strrchr(s, '/');
>+
>+	return p ? p + 1 : s;
>+}
>+
> static int fn_to_prefix(const char *fn, char *prefix, size_t size)
> {
> 	size_t len;
>
>-	fn = basename(fn);
>+	fn = xbasename(fn);
> 	len = strlen(fn);
>
> 	if (len > size - 1)
>-- 
>2.51.0.rc2.233.g662b1ed5c5-goog
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-08-25 16:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-20 20:16 [PATCH] drm/xe: replace basename() with portable strrchr() Carlos Llamas
2025-08-20 21:15 ` Lucas De Marchi
2025-08-20 23:05   ` Carlos Llamas
2025-08-21 21:00     ` Lucas De Marchi
2025-08-21 21:32       ` Carlos Llamas
2025-08-21 22:00       ` [PATCH v2] drm/xe: switch to local __basename() helper Carlos Llamas
2025-08-23 11:56         ` Lucas De Marchi
2025-08-25 15:49           ` Carlos Llamas
2025-08-25 15:57           ` [PATCH v3] drm/xe: switch to local xbasename() helper Carlos Llamas
2025-08-25 16:01             ` Lucas De Marchi
2025-08-20 21:18 ` [PATCH] drm/xe: replace basename() with portable strrchr() Tiffany Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).