[v5,BlueZ] Provide GNU basename compatible implementation

Call to basename() relies on a GNU extension
to take a const char * vs a char *. Let's define
a trivial helper function to ensure compatibility
with musl.

Fixes Issue: https://github.com/bluez/bluez/issues/843
---
v2: Fix code formatter reported errors
v3: Make just node_name as const and keep node_dir as such
v4: Fix code formatting errors
v5: Redo the patch to address textrels seen on ppc32/arm

 configure.ac            | 11 ++++++++++-
 mesh/mesh-config-json.c |  4 +++-
 mesh/missing.h          | 21 +++++++++++++++++++++
 mesh/rpl.c              |  1 +
 tools/hex2hcd.c         |  1 +
 tools/missing.h         | 21 +++++++++++++++++++++
 6 files changed, 57 insertions(+), 2 deletions(-)
 create mode 100644 mesh/missing.h
 create mode 100644 tools/missing.h

Hi Khem,

On Wed, Aug 28, 2024 at 12:17 AM Khem Raj <raj.khem@gmail.com> wrote:
>
> Call to basename() relies on a GNU extension
> to take a const char * vs a char *. Let's define
> a trivial helper function to ensure compatibility
> with musl.
>
> Fixes Issue: https://github.com/bluez/bluez/issues/843
> ---
> v2: Fix code formatter reported errors
> v3: Make just node_name as const and keep node_dir as such
> v4: Fix code formatting errors
> v5: Redo the patch to address textrels seen on ppc32/arm

Not really sure why you went with something completely different then
the util helper?

>  configure.ac            | 11 ++++++++++-
>  mesh/mesh-config-json.c |  4 +++-
>  mesh/missing.h          | 21 +++++++++++++++++++++
>  mesh/rpl.c              |  1 +
>  tools/hex2hcd.c         |  1 +
>  tools/missing.h         | 21 +++++++++++++++++++++
>  6 files changed, 57 insertions(+), 2 deletions(-)
>  create mode 100644 mesh/missing.h
>  create mode 100644 tools/missing.h
>
> diff --git a/configure.ac b/configure.ac
> index d31eb1656..f0f1ec100 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -70,7 +70,16 @@ AC_CHECK_LIB(pthread, pthread_create, dummy=yes,
>  AC_CHECK_LIB(dl, dlopen, dummy=yes,
>                         AC_MSG_ERROR(dynamic linking loader is required))
>
> -AC_CHECK_HEADERS(linux/types.h linux/if_alg.h linux/uinput.h linux/uhid.h sys/random.h)
> +AC_CHECK_HEADERS(string.h linux/types.h linux/if_alg.h linux/uinput.h linux/uhid.h sys/random.h)
> +
> +# basename may be only available in libgen.h with the POSIX behavior,
> +# not desired here
> +AC_CHECK_DECLS([basename], [],
> +                               AC_MSG_WARN([GNU basename extension not found]),
> +                               [#define _GNU_SOURCE 1
> +                                #include <string.h>
> +                                ])
> +
>
>  PKG_CHECK_MODULES(GLIB, glib-2.0 >= 2.28)
>
> diff --git a/mesh/mesh-config-json.c b/mesh/mesh-config-json.c
> index c198627c6..e3b0a1809 100644
> --- a/mesh/mesh-config-json.c
> +++ b/mesh/mesh-config-json.c
> @@ -28,6 +28,7 @@
>  #include <ell/ell.h>
>  #include <json-c/json.h>
>
> +#include "mesh/missing.h"
>  #include "mesh/mesh-defs.h"
>  #include "mesh/util.h"
>  #include "mesh/mesh-config.h"
> @@ -2694,7 +2695,8 @@ bool mesh_config_load_nodes(const char *cfgdir_name, mesh_config_node_func_t cb,
>
>  void mesh_config_destroy_nvm(struct mesh_config *cfg)
>  {
> -       char *node_dir, *node_name;
> +       char *node_dir;
> +       const char* node_name;
>         char uuid[33];
>
>         if (!cfg)
> diff --git a/mesh/missing.h b/mesh/missing.h
> new file mode 100644
> index 000000000..eaf32815e
> --- /dev/null
> +++ b/mesh/missing.h
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: LGPL-2.1-or-later
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2024  Khem Raj <raj.khem@gmail.com>
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +#if !HAVE_DECL_BASENAME
> +#include <string.h>
> +static inline const char *basename(const char *path)
> +{
> +  const char *base = strrchr(path, '/');
> +
> +  return base ? base + 1 : path;
> +}
> +#endif
> diff --git a/mesh/rpl.c b/mesh/rpl.c
> index fb225dddd..2fa17d72f 100644
> --- a/mesh/rpl.c
> +++ b/mesh/rpl.c
> @@ -24,6 +24,7 @@
>
>  #include <ell/ell.h>
>
> +#include "mesh/missing.h"
>  #include "mesh/mesh-defs.h"
>
>  #include "mesh/node.h"
> diff --git a/tools/hex2hcd.c b/tools/hex2hcd.c
> index e6dca5a81..452ab2beb 100644
> --- a/tools/hex2hcd.c
> +++ b/tools/hex2hcd.c
> @@ -24,6 +24,7 @@
>  #include <stdlib.h>
>  #include <stdbool.h>
>  #include <sys/stat.h>
> +#include "tools/missing.h"
>
>  static ssize_t process_record(int fd, const char *line, uint16_t *upper_addr)
>  {
> diff --git a/tools/missing.h b/tools/missing.h
> new file mode 100644
> index 000000000..eaf32815e
> --- /dev/null
> +++ b/tools/missing.h
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: LGPL-2.1-or-later
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2024  Khem Raj <raj.khem@gmail.com>
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +#if !HAVE_DECL_BASENAME
> +#include <string.h>
> +static inline const char *basename(const char *path)
> +{
> +  const char *base = strrchr(path, '/');
> +
> +  return base ? base + 1 : path;
> +}
> +#endif
>

On Wed, Aug 28, 2024 at 7:50 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Khem,
>
> On Wed, Aug 28, 2024 at 10:20 AM Khem Raj <raj.khem@gmail.com> wrote:
> >
> > On Wed, Aug 28, 2024 at 7:07 AM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > Hi Khem,
> > >
> > > On Wed, Aug 28, 2024 at 12:17 AM Khem Raj <raj.khem@gmail.com> wrote:
> > > >
> > > > Call to basename() relies on a GNU extension
> > > > to take a const char * vs a char *. Let's define
> > > > a trivial helper function to ensure compatibility
> > > > with musl.
> > > >
> > > > Fixes Issue: https://github.com/bluez/bluez/issues/843
> > > > ---
> > > > v2: Fix code formatter reported errors
> > > > v3: Make just node_name as const and keep node_dir as such
> > > > v4: Fix code formatting errors
> > > > v5: Redo the patch to address textrels seen on ppc32/arm
> > >
> > > Not really sure why you went with something completely different then
> > > the util helper?
> >
> > util helper meant that we need to add -I option to build the utilities
> > needing this function from util,h
> > and this -I was causing more stuff to be included into these binaries,
> > It got caught when building for ppc32
> > since yocto build started failing due to textrels and I realized that
> > specifying -I can open up the possibility of
> > wrong includes coming from src/ dir. Thats the reason I went with the
> > alternative approach which is safer
> > although the function is copied twice.
>
> Where did you see this error? Perhaps we need to include such a build
> into our own CI then, anyway it seems fair enough but Id add some
> comment about it, also the other possibility would be to add something
> like l_path_basename to libell but I guess that comes with its own
> problem since we would need to uprev the ell dependency.

building hex2hcd for ppc32 target using pie enabled revealed the problem
for yocto, not sure if thats something of interest. I will add a
reasoning to not
add it to util.h in v7.

>
> >
> > >
> > > >  configure.ac            | 11 ++++++++++-
> > > >  mesh/mesh-config-json.c |  4 +++-
> > > >  mesh/missing.h          | 21 +++++++++++++++++++++
> > > >  mesh/rpl.c              |  1 +
> > > >  tools/hex2hcd.c         |  1 +
> > > >  tools/missing.h         | 21 +++++++++++++++++++++
> > > >  6 files changed, 57 insertions(+), 2 deletions(-)
> > > >  create mode 100644 mesh/missing.h
> > > >  create mode 100644 tools/missing.h
> > > >
> > > > diff --git a/configure.ac b/configure.ac
> > > > index d31eb1656..f0f1ec100 100644
> > > > --- a/configure.ac
> > > > +++ b/configure.ac
> > > > @@ -70,7 +70,16 @@ AC_CHECK_LIB(pthread, pthread_create, dummy=yes,
> > > >  AC_CHECK_LIB(dl, dlopen, dummy=yes,
> > > >                         AC_MSG_ERROR(dynamic linking loader is required))
> > > >
> > > > -AC_CHECK_HEADERS(linux/types.h linux/if_alg.h linux/uinput.h linux/uhid.h sys/random.h)
> > > > +AC_CHECK_HEADERS(string.h linux/types.h linux/if_alg.h linux/uinput.h linux/uhid.h sys/random.h)
> > > > +
> > > > +# basename may be only available in libgen.h with the POSIX behavior,
> > > > +# not desired here
> > > > +AC_CHECK_DECLS([basename], [],
> > > > +                               AC_MSG_WARN([GNU basename extension not found]),
> > > > +                               [#define _GNU_SOURCE 1
> > > > +                                #include <string.h>
> > > > +                                ])
> > > > +
> > > >
> > > >  PKG_CHECK_MODULES(GLIB, glib-2.0 >= 2.28)
> > > >
> > > > diff --git a/mesh/mesh-config-json.c b/mesh/mesh-config-json.c
> > > > index c198627c6..e3b0a1809 100644
> > > > --- a/mesh/mesh-config-json.c
> > > > +++ b/mesh/mesh-config-json.c
> > > > @@ -28,6 +28,7 @@
> > > >  #include <ell/ell.h>
> > > >  #include <json-c/json.h>
> > > >
> > > > +#include "mesh/missing.h"
> > > >  #include "mesh/mesh-defs.h"
> > > >  #include "mesh/util.h"
> > > >  #include "mesh/mesh-config.h"
> > > > @@ -2694,7 +2695,8 @@ bool mesh_config_load_nodes(const char *cfgdir_name, mesh_config_node_func_t cb,
> > > >
> > > >  void mesh_config_destroy_nvm(struct mesh_config *cfg)
> > > >  {
> > > > -       char *node_dir, *node_name;
> > > > +       char *node_dir;
> > > > +       const char* node_name;
> > > >         char uuid[33];
> > > >
> > > >         if (!cfg)
> > > > diff --git a/mesh/missing.h b/mesh/missing.h
> > > > new file mode 100644
> > > > index 000000000..eaf32815e
> > > > --- /dev/null
> > > > +++ b/mesh/missing.h
> > > > @@ -0,0 +1,21 @@
> > > > +// SPDX-License-Identifier: LGPL-2.1-or-later
> > > > +/*
> > > > + *
> > > > + *  BlueZ - Bluetooth protocol stack for Linux
> > > > + *
> > > > + *  Copyright (C) 2024  Khem Raj <raj.khem@gmail.com>
> > > > + *
> > > > + */
> > > > +
> > > > +#ifdef HAVE_CONFIG_H
> > > > +#include <config.h>
> > > > +#endif
> > > > +#if !HAVE_DECL_BASENAME
> > > > +#include <string.h>
> > > > +static inline const char *basename(const char *path)
> > > > +{
> > > > +  const char *base = strrchr(path, '/');
> > > > +
> > > > +  return base ? base + 1 : path;
> > > > +}
> > > > +#endif
> > > > diff --git a/mesh/rpl.c b/mesh/rpl.c
> > > > index fb225dddd..2fa17d72f 100644
> > > > --- a/mesh/rpl.c
> > > > +++ b/mesh/rpl.c
> > > > @@ -24,6 +24,7 @@
> > > >
> > > >  #include <ell/ell.h>
> > > >
> > > > +#include "mesh/missing.h"
> > > >  #include "mesh/mesh-defs.h"
> > > >
> > > >  #include "mesh/node.h"
> > > > diff --git a/tools/hex2hcd.c b/tools/hex2hcd.c
> > > > index e6dca5a81..452ab2beb 100644
> > > > --- a/tools/hex2hcd.c
> > > > +++ b/tools/hex2hcd.c
> > > > @@ -24,6 +24,7 @@
> > > >  #include <stdlib.h>
> > > >  #include <stdbool.h>
> > > >  #include <sys/stat.h>
> > > > +#include "tools/missing.h"
> > > >
> > > >  static ssize_t process_record(int fd, const char *line, uint16_t *upper_addr)
> > > >  {
> > > > diff --git a/tools/missing.h b/tools/missing.h
> > > > new file mode 100644
> > > > index 000000000..eaf32815e
> > > > --- /dev/null
> > > > +++ b/tools/missing.h
> > > > @@ -0,0 +1,21 @@
> > > > +// SPDX-License-Identifier: LGPL-2.1-or-later
> > > > +/*
> > > > + *
> > > > + *  BlueZ - Bluetooth protocol stack for Linux
> > > > + *
> > > > + *  Copyright (C) 2024  Khem Raj <raj.khem@gmail.com>
> > > > + *
> > > > + */
> > > > +
> > > > +#ifdef HAVE_CONFIG_H
> > > > +#include <config.h>
> > > > +#endif
> > > > +#if !HAVE_DECL_BASENAME
> > > > +#include <string.h>
> > > > +static inline const char *basename(const char *path)
> > > > +{
> > > > +  const char *base = strrchr(path, '/');
> > > > +
> > > > +  return base ? base + 1 : path;
> > > > +}
> > > > +#endif
> > > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz