containers / composefs

a file system for mounting container images
GNU General Public License v2.0
421 stars 29 forks source link

musl: basename: use portable implementation for basename API #273

Closed fboudra closed 4 months ago

fboudra commented 5 months ago

musl has removed the non-prototype declaration of basename from string.h which now results in build errors with newer clang compilers.

Implement GNU basename behavior using strchr which is portable across libcs.

Fixes: | ../../git/tools/mountcomposefs.c:43:20: | error: call to undeclared function 'basename'; ISO C99 and later do not | support implicit function declarations [-Wimplicit-function-declaration] | 43 | const char bin = basename(argv0); | | ^ | ../../git/tools/mountcomposefs.c:43:14: | error: incompatible integer to pointer conversion initializing 'const char ' | with an expression of type 'int' [-Wint-conversion] | 43 | const char *bin = basename(argv0); | | ^ ~~~

For reference: https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7

Closes: https://github.com/containers/composefs/issues/272

quaresmajose commented 5 months ago

maybe just the #include <libgen.h> can do the job.

it includes the posix implementation of the basename https://man7.org/linux/man-pages/man0/libgen.h.0p.html

fboudra commented 5 months ago

@quaresmajose I tried it initially. It looks like we need to use the gnu implementation in this case, not the posix one (including libgen.h won't be enough). Hence, I ended to propose the approach used in kmod. Pending on the authors to review and see if it's acceptable to them.

quaresmajose commented 5 months ago

Ok, understand. My suggestion was mainly because I saw several patches for this issue and many uses the libgen.h. Thanks for the clarification.

alexlarsson commented 5 months ago

Looks fine to me, I'm not overly excited about carrying re-implementations like this, but its trivial enough to not be worth caring about.

alexlarsson commented 5 months ago

Ah, seems to be some formating issues, can you run "make clang-format" and repush?

fboudra commented 5 months ago

@alexlarsson thanks. formating issue fixed and repushed.