[PATCH] ipu3: Use posix basename

[Kieran Bingham]

Quoting Laurent Pinchart (2024-04-16 16:05:51)
> On Tue, Apr 16, 2024 at 03:58:53PM +0100, Kieran Bingham wrote:
> > Quoting Khem Raj (2024-03-24 18:33:07)
> > > musl does not implement GNU basename extention and with latest musl
> > > the prototype from string.h is also removed [1] which now results in
> > > compile errors e.g.
> > > 
> > > ../git/utils/ipu3/ipu3-pack.c:21:47: error: call to undeclared function 'basename'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> > > 
> > > These utilities are using this function in usage() which is used just
> > > before program exit. Always use the basename APIs from libgen.h which is
> > > posix implementation
> > > 
> > > [1] https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7
> > 
> > This passes all of our compile and lint tests presently. We don't have a
> > Musl target in CI yet. Perhaps we should add one.
> > 
> > https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1138070
> > 
> > No particularly strong opinion on the patch, but it fixes an issue
> > for you, and is only in one of the utils, and doesn't break CI ...
> > 
> > 
> > It's a shame that this removes a const on a variable that otherwise
> > shouldn't be modified but ... I think we can cope with that here.
> > 
> > In fact we have our own implementation of basename() in our base library
> > utils probably because of this reason. But I don't think linking against
> > libcamera-base is really required or desired/necessary for the
> > ipu3-pack/unpack utils so ...
> 
> Is there a real need to compile ipu3-pack and ipu3-unpack on musl-based
> systems ? Those are development tools that nobody has used for ages.

Maybe not, but regardless they are built as part of the whole libcamera
build, so ensureing we continue to compile is probably a requirement.

Or we drop the utils... but I think that would be unfortunate for anyone
who then later jumps on to the ipu3 and wants to play around with this.

But that's exactly why dropping a const here seems ... quite minor.

Do you prefer to nak this patch? Or maybe disable the build? (but let it
bit rot?)

--
Kieran



> 
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> > > Signed-off-by: Khem Raj <raj.khem at gmail.com>
> > > ---
> > >  utils/ipu3/ipu3-pack.c   | 4 ++--
> > >  utils/ipu3/ipu3-unpack.c | 3 ++-
> > >  2 files changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/utils/ipu3/ipu3-pack.c b/utils/ipu3/ipu3-pack.c
> > > index decbfc6c..23d2db8b 100644
> > > --- a/utils/ipu3/ipu3-pack.c
> > > +++ b/utils/ipu3/ipu3-pack.c
> > > @@ -8,6 +8,7 @@
> > >  
> > >  #include <errno.h>
> > >  #include <fcntl.h>
> > > +#include <libgen.h>
> > >  #include <stdint.h>
> > >  #include <stdio.h>
> > >  #include <string.h>
> > > @@ -15,9 +16,8 @@
> > >  #include <sys/types.h>
> > >  #include <unistd.h>
> > >  
> > > -static void usage(const char *argv0)
> > > +static void usage(char *argv0)
> > >  {
> > > -
> > >         printf("Usage: %s input-file output-file\n", basename(argv0));
> > >         printf("Convert unpacked RAW10 Bayer data to the IPU3 packed Bayer formats\n");
> > >         printf("If the output-file '-', output data will be written to standard output\n");
> > > diff --git a/utils/ipu3/ipu3-unpack.c b/utils/ipu3/ipu3-unpack.c
> > > index 9d2c1200..1505a970 100644
> > > --- a/utils/ipu3/ipu3-unpack.c
> > > +++ b/utils/ipu3/ipu3-unpack.c
> > > @@ -8,6 +8,7 @@
> > >  
> > >  #include <errno.h>
> > >  #include <fcntl.h>
> > > +#include <libgen.h>
> > >  #include <stdint.h>
> > >  #include <stdio.h>
> > >  #include <string.h>
> > > @@ -15,7 +16,7 @@
> > >  #include <sys/types.h>
> > >  #include <unistd.h>
> > >  
> > > -static void usage(const char *argv0)
> > > +static void usage(char *argv0)
> > >  {
> > >         printf("Usage: %s input-file output-file\n", basename(argv0));
> > >         printf("Unpack the IPU3 raw Bayer format to 16-bit Bayer\n");
> 
> -- 
> Regards,
> 
> Laurent Pinchart