| Message ID | 20240828041644.3331363-1-raj.khem@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5,BlueZ] Provide GNU basename compatible implementation | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | fail | ERROR:POINTER_LOCATION: "foo* bar" should be "foo *bar" #151: FILE: mesh/mesh-config-json.c:2699: + const char* node_name; WARNING:LEADING_SPACE: please, no spaces at the start of a line #177: FILE: mesh/missing.h:17: + const char *base = strrchr(path, '/');$ WARNING:LEADING_SPACE: please, no spaces at the start of a line #179: FILE: mesh/missing.h:19: + return base ? base + 1 : path;$ WARNING:LEADING_SPACE: please, no spaces at the start of a line #228: FILE: tools/missing.h:17: + const char *base = strrchr(path, '/');$ WARNING:LEADING_SPACE: please, no spaces at the start of a line #230: FILE: tools/missing.h:19: + return base ? base + 1 : path;$ /github/workspace/src/src/13780500.patch total: 1 errors, 4 warnings, 89 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13780500.patch has style problems, please review. NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | fail | Make Distcheck FAIL: Package cups was not found in the pkg-config search path. Perhaps you should add the directory containing `cups.pc' to the PKG_CONFIG_PATH environment variable No package 'cups' found ../../mesh/mesh-config-json.c:31:10: fatal error: mesh/missing.h: No such file or directory 31 | #include "mesh/missing.h" | ^~~~~~~~~~~~~~~~ compilation terminated. make[2]: *** [Makefile:7850: mesh/mesh-config-json.o] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [Makefile:4676: all] Error 2 make: *** [Makefile:12226: distcheck] Error 1 |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | warning | CheckSparse WARNING tools/hex2hcd.c:136:26: warning: Variable length array is used. |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=884060
---Test result---
Test Summary:
CheckPatch FAIL 0.63 seconds
GitLint PASS 0.27 seconds
BuildEll PASS 24.68 seconds
BluezMake PASS 1683.62 seconds
MakeCheck PASS 13.42 seconds
MakeDistcheck FAIL 68.17 seconds
CheckValgrind PASS 253.06 seconds
CheckSmatch WARNING 357.07 seconds
bluezmakeextell PASS 122.25 seconds
IncrementalBuild PASS 1465.75 seconds
ScanBuild PASS 1043.98 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[v5,BlueZ] Provide GNU basename compatible implementation
ERROR:POINTER_LOCATION: "foo* bar" should be "foo *bar"
#151: FILE: mesh/mesh-config-json.c:2699:
+ const char* node_name;
WARNING:LEADING_SPACE: please, no spaces at the start of a line
#177: FILE: mesh/missing.h:17:
+ const char *base = strrchr(path, '/');$
WARNING:LEADING_SPACE: please, no spaces at the start of a line
#179: FILE: mesh/missing.h:19:
+ return base ? base + 1 : path;$
WARNING:LEADING_SPACE: please, no spaces at the start of a line
#228: FILE: tools/missing.h:17:
+ const char *base = strrchr(path, '/');$
WARNING:LEADING_SPACE: please, no spaces at the start of a line
#230: FILE: tools/missing.h:19:
+ return base ? base + 1 : path;$
/github/workspace/src/src/13780500.patch total: 1 errors, 4 warnings, 89 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13780500.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: MakeDistcheck - FAIL
Desc: Run Bluez Make Distcheck
Output:
Package cups was not found in the pkg-config search path.
Perhaps you should add the directory containing `cups.pc'
to the PKG_CONFIG_PATH environment variable
No package 'cups' found
../../mesh/mesh-config-json.c:31:10: fatal error: mesh/missing.h: No such file or directory
31 | #include "mesh/missing.h"
| ^~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [Makefile:7850: mesh/mesh-config-json.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [Makefile:4676: all] Error 2
make: *** [Makefile:12226: distcheck] Error 1
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
tools/hex2hcd.c:136:26: warning: Variable length array is used.
---
Regards,
Linux Bluetooth
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: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. > > > 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
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. > > > > > > 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
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
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