[PATCH v2] drm/xe: switch to local __basename() helper

Carlos Llamas posted 1 patch 1 month, 1 week ago
drivers/gpu/drm/xe/xe_gen_wa_oob.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
[PATCH v2] drm/xe: switch to local __basename() helper
Posted by Carlos Llamas 1 month, 1 week ago
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
Re: [PATCH v2] drm/xe: switch to local __basename() helper
Posted by Lucas De Marchi 1 month, 1 week ago
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
>
Re: [PATCH v2] drm/xe: switch to local __basename() helper
Posted by Carlos Llamas 1 month, 1 week ago
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...
[PATCH v3] drm/xe: switch to local xbasename() helper
Posted by Carlos Llamas 1 month, 1 week ago
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
Re: [PATCH v3] drm/xe: switch to local xbasename() helper
Posted by Lucas De Marchi 1 month, 1 week ago
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
>