Re: [PATCH] include libgen.h for basename API

[Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>]

On Sun, 24 Mar 2024 23:15:37 -0700
Khem Raj <raj.khem@gmail.com> wrote:

> Musl does no more provide it via string.h therefore builds with newer
> compilers e.g. clang-18 fails due to missing prototype for basename
> therefore add libgen.h to included headers list
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>  Monitor.c        | 1 +
>  platform-intel.c | 1 +
>  super-intel.c    | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/Monitor.c b/Monitor.c
> index 824a69f..e3942e1 100644
> --- a/Monitor.c
> +++ b/Monitor.c
> @@ -26,6 +26,7 @@
>  #include	"udev.h"
>  #include	"md_p.h"
>  #include	"md_u.h"
> +#include  <libgen.h>
>  #include	<sys/wait.h>
>  #include	<limits.h>
>  #include	<syslog.h>

Please stick to style around.

> diff --git a/platform-intel.c b/platform-intel.c
> index ac282bc..5d6687d 100644
> --- a/platform-intel.c
> +++ b/platform-intel.c
> @@ -19,6 +19,7 @@
>  #include "mdadm.h"
>  #include "platform-intel.h"
>  #include "probe_roms.h"
> +#include <libgen.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> diff --git a/super-intel.c b/super-intel.c
> index dbea235..881dbda 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -23,6 +23,7 @@
>  #include "dlink.h"
>  #include "sha1.h"
>  #include "platform-intel.h"
> +#include <libgen.h>
>  #include <values.h>
>  #include <scsi/sg.h>
>  #include <ctype.h>

https://man7.org/linux/man-pages/man3/basename.3.html

       There are two different versions of basename() - the POSIX
       version described above, and the GNU version [...]

       In the glibc implementation, the POSIX versions of these
       functions modify the path argument, and segfault when called with
       a static string such as "/usr/".

I checked my string.h header (opensuse 15 sp4) and there is:

/* Return the file name within directory of FILENAME.  We don't
   declare the function if the `basename' macro is available (defined
   in <libgen.h>) which makes the XPG version of this function
   available.  */

It means that with libgen.h header we will switch to XPG/POSIX basename
everywhere, even if that is not wanted.

Eventually, you can try to minimize impact, by adding it only if it is
absolutely necessary i.e. basename() is not available:

#ifndef basename
   #include <libgen.h>
#endif
(not tested, partially copied from string.h)

Please analyze all usages to determine if POSIX version of basename usages are
safe- I would like to have that documented in commit message.

Thanks,
Mariusz