[libcamera-devel] [PATCH] utils: ipu3: Include libgen.h for basename()

[Jacopo Mondi]

Hi Laurent

On Fri, Dec 02, 2022 at 03:59:07PM +0200, Laurent Pinchart wrote:
> On Fri, Dec 02, 2022 at 02:48:27PM +0100, Jacopo Mondi wrote:
> > Not really...
> >
> > I get with clang
> >
> > ../utils/ipu3/ipu3-unpack.c:21:63: error: passing argument 1 of ‘__xpg_basename’ discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
> >    21 |         printf("Usage: %s input-file output-file\n", basename(argv0));
> >
> > Sorry, I should have tested before, but it seemed trivial.
> >
> > I now wonder what 'basename' we were calling, or better, to which
> > function prototype we were referring to, which didn't trigger such
> > error...
>
> From libgen.h:
>
> /* Return final component of PATH.
>
>    This is the weird XPG version of this function.  It sometimes will
>    modify its argument.  Therefore we normally use the GNU version (in
>    <string.h>) and only if this header is included make the XPG
>    version available under the real name.  */
> extern char *__xpg_basename (char *__path) __THROW;
> #define basename        __xpg_basename
>
> So we're using the string.h version. The basename manpage mentions

Spot on, I read the header file after sending the email....

>
> NOTES
>        There are two different versions of basename() - the POSIX version described above, and the GNU version, which one gets after
>
>                #define _GNU_SOURCE         /* See feature_test_macros(7) */
>                #include <string.h>
>
>        The GNU version never modifies its argument, and returns the empty string when path has a trailing slash, and in particular also when it is "/".  There is  no  GNU  version  of
>        dirname().
>
>        With glibc, one gets the POSIX version of basename() when <libgen.h> is included, and the GNU version otherwise.
>
> We have utils::basename() to support different libc versions, but we
> can't use it in utils/. Have you encountered this when compiling for
> Android ? Maybe we can disable compilation of those utilities instead ?
>

Yes, I am trying compile with the NDK.

I think we can safely remove those scripts from the build (I wonder
how this would be decided), but I anticipate already that we'll have a
few hickups as in example:

../src/libcamera/media_device.cpp:167:2: error: use of undeclared identifier 'lockf'
        lockf(fd_.get(), F_ULOCK, 0);

As Android has instead the BSD-conformant flock(). So expect more
wrappers in utils::  (hopefully not many)

> > On Fri, Dec 02, 2022 at 03:33:56PM +0200, Laurent Pinchart wrote:
> > > Hi Jacopo,
> > >
> > > Thank you for the patch.
> > >
> > > On Fri, Dec 02, 2022 at 02:27:06PM +0100, Jacopo Mondi via libcamera-devel wrote:
> > > > The "basename" function from the standard C library is defined in the
> > > > libgen.h header.
> > > >
> > > > Include it to avoid errors triggered by -Wimplicit-function-declaration
> > > >
> > > > ../utils/ipu3/ipu3-pack.c:21:47: error: implicit declaration of function
> > > > 'basename' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > >
> > > > ---
> > > >  utils/ipu3/ipu3-pack.c   | 1 +
> > > >  utils/ipu3/ipu3-unpack.c | 1 +
> > > >  2 files changed, 2 insertions(+)
> > > >
> > > > diff --git a/utils/ipu3/ipu3-pack.c b/utils/ipu3/ipu3-pack.c
> > > > index decbfc6c397a..27af44068956 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>
> > > > diff --git a/utils/ipu3/ipu3-unpack.c b/utils/ipu3/ipu3-unpack.c
> > > > index 9d2c1200d932..aa67e0b0ecf5 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>
>
> --
> Regards,
>
> Laurent Pinchart