[{"id":29234,"web_url":"https://patchwork.libcamera.org/comment/29234/","msgid":"<171327953309.342316.7093064182795030881@ping.linuxembedded.co.uk>","date":"2024-04-16T14:58:53","subject":"Re: [PATCH] ipu3: Use posix basename","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Khem Raj (2024-03-24 18:33:07)\n> musl does not implement GNU basename extention and with latest musl\n> the prototype from string.h is also removed [1] which now results in\n> compile errors e.g.\n> \n> ../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]\n> \n> These utilities are using this function in usage() which is used just\n> before program exit. Always use the basename APIs from libgen.h which is\n> posix implementation\n> \n> [1] https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7\n\nThis passes all of our compile and lint tests presently. We don't have a\nMusl target in CI yet. Perhaps we should add one.\n\nhttps://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1138070\n\nNo particularly strong opinion on the patch, but it fixes an issue\nfor you, and is only in one of the utils, and doesn't break CI ...\n\n\nIt's a shame that this removes a const on a variable that otherwise\nshouldn't be modified but ... I think we can cope with that here.\n\nIn fact we have our own implementation of basename() in our base library\nutils probably because of this reason. But I don't think linking against\nlibcamera-base is really required or desired/necessary for the\nipu3-pack/unpack utils so ...\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> \n> Signed-off-by: Khem Raj <raj.khem@gmail.com>\n> ---\n>  utils/ipu3/ipu3-pack.c   | 4 ++--\n>  utils/ipu3/ipu3-unpack.c | 3 ++-\n>  2 files changed, 4 insertions(+), 3 deletions(-)\n> \n> diff --git a/utils/ipu3/ipu3-pack.c b/utils/ipu3/ipu3-pack.c\n> index decbfc6c..23d2db8b 100644\n> --- a/utils/ipu3/ipu3-pack.c\n> +++ b/utils/ipu3/ipu3-pack.c\n> @@ -8,6 +8,7 @@\n>  \n>  #include <errno.h>\n>  #include <fcntl.h>\n> +#include <libgen.h>\n>  #include <stdint.h>\n>  #include <stdio.h>\n>  #include <string.h>\n> @@ -15,9 +16,8 @@\n>  #include <sys/types.h>\n>  #include <unistd.h>\n>  \n> -static void usage(const char *argv0)\n> +static void usage(char *argv0)\n>  {\n> -\n>         printf(\"Usage: %s input-file output-file\\n\", basename(argv0));\n>         printf(\"Convert unpacked RAW10 Bayer data to the IPU3 packed Bayer formats\\n\");\n>         printf(\"If the output-file '-', output data will be written to standard output\\n\");\n> diff --git a/utils/ipu3/ipu3-unpack.c b/utils/ipu3/ipu3-unpack.c\n> index 9d2c1200..1505a970 100644\n> --- a/utils/ipu3/ipu3-unpack.c\n> +++ b/utils/ipu3/ipu3-unpack.c\n> @@ -8,6 +8,7 @@\n>  \n>  #include <errno.h>\n>  #include <fcntl.h>\n> +#include <libgen.h>\n>  #include <stdint.h>\n>  #include <stdio.h>\n>  #include <string.h>\n> @@ -15,7 +16,7 @@\n>  #include <sys/types.h>\n>  #include <unistd.h>\n>  \n> -static void usage(const char *argv0)\n> +static void usage(char *argv0)\n>  {\n>         printf(\"Usage: %s input-file output-file\\n\", basename(argv0));\n>         printf(\"Unpack the IPU3 raw Bayer format to 16-bit Bayer\\n\");\n> -- \n> 2.44.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BFAC6C3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 16 Apr 2024 14:58:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A158C63352;\n\tTue, 16 Apr 2024 16:58:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6431061B75\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 16 Apr 2024 16:58:55 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C3C00DFB;\n\tTue, 16 Apr 2024 16:58:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vUgTcHDd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713279488;\n\tbh=WqtuAZlirvi/szTZEfkLeT2tkGv1mtZJpsu2jp8nn38=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=vUgTcHDday8p0Mja0N05juhRv6FFLkoaumhQ1Cr0ubHOCsFOkhOKcv+LHmL/hL45m\n\tFe7T3g+AVIGkg3yP2HuHkg9xg/u864TXb7uablPMy4VyYUcZ+0cZd5rXfXuHfNqiew\n\tu94oP3FWzooUf85TbW6VnbXaREqOeta+B2JYLj4c=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240324183307.3833076-1-raj.khem@gmail.com>","References":"<20240324183307.3833076-1-raj.khem@gmail.com>","Subject":"Re: [PATCH] ipu3: Use posix basename","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"laurent.pinchart@ideasonboard.com, Khem Raj <raj.khem@gmail.com>","To":"Khem Raj <raj.khem@gmail.com>, libcamera-devel@lists.libcamera.org","Date":"Tue, 16 Apr 2024 15:58:53 +0100","Message-ID":"<171327953309.342316.7093064182795030881@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29235,"web_url":"https://patchwork.libcamera.org/comment/29235/","msgid":"<20240416150551.GI12561@pendragon.ideasonboard.com>","date":"2024-04-16T15:05:51","subject":"Re: [PATCH] ipu3: Use posix basename","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Apr 16, 2024 at 03:58:53PM +0100, Kieran Bingham wrote:\n> Quoting Khem Raj (2024-03-24 18:33:07)\n> > musl does not implement GNU basename extention and with latest musl\n> > the prototype from string.h is also removed [1] which now results in\n> > compile errors e.g.\n> > \n> > ../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]\n> > \n> > These utilities are using this function in usage() which is used just\n> > before program exit. Always use the basename APIs from libgen.h which is\n> > posix implementation\n> > \n> > [1] https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7\n> \n> This passes all of our compile and lint tests presently. We don't have a\n> Musl target in CI yet. Perhaps we should add one.\n> \n> https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1138070\n> \n> No particularly strong opinion on the patch, but it fixes an issue\n> for you, and is only in one of the utils, and doesn't break CI ...\n> \n> \n> It's a shame that this removes a const on a variable that otherwise\n> shouldn't be modified but ... I think we can cope with that here.\n> \n> In fact we have our own implementation of basename() in our base library\n> utils probably because of this reason. But I don't think linking against\n> libcamera-base is really required or desired/necessary for the\n> ipu3-pack/unpack utils so ...\n\nIs there a real need to compile ipu3-pack and ipu3-unpack on musl-based\nsystems ? Those are development tools that nobody has used for ages.\n\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> > Signed-off-by: Khem Raj <raj.khem@gmail.com>\n> > ---\n> >  utils/ipu3/ipu3-pack.c   | 4 ++--\n> >  utils/ipu3/ipu3-unpack.c | 3 ++-\n> >  2 files changed, 4 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/utils/ipu3/ipu3-pack.c b/utils/ipu3/ipu3-pack.c\n> > index decbfc6c..23d2db8b 100644\n> > --- a/utils/ipu3/ipu3-pack.c\n> > +++ b/utils/ipu3/ipu3-pack.c\n> > @@ -8,6 +8,7 @@\n> >  \n> >  #include <errno.h>\n> >  #include <fcntl.h>\n> > +#include <libgen.h>\n> >  #include <stdint.h>\n> >  #include <stdio.h>\n> >  #include <string.h>\n> > @@ -15,9 +16,8 @@\n> >  #include <sys/types.h>\n> >  #include <unistd.h>\n> >  \n> > -static void usage(const char *argv0)\n> > +static void usage(char *argv0)\n> >  {\n> > -\n> >         printf(\"Usage: %s input-file output-file\\n\", basename(argv0));\n> >         printf(\"Convert unpacked RAW10 Bayer data to the IPU3 packed Bayer formats\\n\");\n> >         printf(\"If the output-file '-', output data will be written to standard output\\n\");\n> > diff --git a/utils/ipu3/ipu3-unpack.c b/utils/ipu3/ipu3-unpack.c\n> > index 9d2c1200..1505a970 100644\n> > --- a/utils/ipu3/ipu3-unpack.c\n> > +++ b/utils/ipu3/ipu3-unpack.c\n> > @@ -8,6 +8,7 @@\n> >  \n> >  #include <errno.h>\n> >  #include <fcntl.h>\n> > +#include <libgen.h>\n> >  #include <stdint.h>\n> >  #include <stdio.h>\n> >  #include <string.h>\n> > @@ -15,7 +16,7 @@\n> >  #include <sys/types.h>\n> >  #include <unistd.h>\n> >  \n> > -static void usage(const char *argv0)\n> > +static void usage(char *argv0)\n> >  {\n> >         printf(\"Usage: %s input-file output-file\\n\", basename(argv0));\n> >         printf(\"Unpack the IPU3 raw Bayer format to 16-bit Bayer\\n\");","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D701CBE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 16 Apr 2024 15:06:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0AEE56333F;\n\tTue, 16 Apr 2024 17:06:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 81DAF61B75\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 16 Apr 2024 17:06:00 +0200 (CEST)","from pendragon.ideasonboard.com (ptr-212-224-235-74.dyn.orange.be\n\t[212.224.235.74])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AFEAB480;\n\tTue, 16 Apr 2024 17:05:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"HH0bRIvU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713279913;\n\tbh=6JnIuQKOuPH8ajHIxg/vEF/kAssh9nrN0t7VMfBPV5k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HH0bRIvUxq3/qYvFSfjDxEPDJs4vXYsCgMsNoXGzOmtJEEn4IPpiBs/eTmdKblpJd\n\t8PVfPPMyely9HssvRnemZVHaWJnVvK9hdFsD/Zvf9fD1z+YpomhsalHw7D+/qbAy9n\n\tcnKcq4SrwDYbWLOhaZjlmkkwU1zbYmsVkTc4H8X8=","Date":"Tue, 16 Apr 2024 18:05:51 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Khem Raj <raj.khem@gmail.com>, libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] ipu3: Use posix basename","Message-ID":"<20240416150551.GI12561@pendragon.ideasonboard.com>","References":"<20240324183307.3833076-1-raj.khem@gmail.com>\n\t<171327953309.342316.7093064182795030881@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<171327953309.342316.7093064182795030881@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29238,"web_url":"https://patchwork.libcamera.org/comment/29238/","msgid":"<171328505179.2468918.6974895004700654332@ping.linuxembedded.co.uk>","date":"2024-04-16T16:30:51","subject":"Re: [PATCH] ipu3: Use posix basename","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2024-04-16 16:05:51)\n> On Tue, Apr 16, 2024 at 03:58:53PM +0100, Kieran Bingham wrote:\n> > Quoting Khem Raj (2024-03-24 18:33:07)\n> > > musl does not implement GNU basename extention and with latest musl\n> > > the prototype from string.h is also removed [1] which now results in\n> > > compile errors e.g.\n> > > \n> > > ../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]\n> > > \n> > > These utilities are using this function in usage() which is used just\n> > > before program exit. Always use the basename APIs from libgen.h which is\n> > > posix implementation\n> > > \n> > > [1] https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7\n> > \n> > This passes all of our compile and lint tests presently. We don't have a\n> > Musl target in CI yet. Perhaps we should add one.\n> > \n> > https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1138070\n> > \n> > No particularly strong opinion on the patch, but it fixes an issue\n> > for you, and is only in one of the utils, and doesn't break CI ...\n> > \n> > \n> > It's a shame that this removes a const on a variable that otherwise\n> > shouldn't be modified but ... I think we can cope with that here.\n> > \n> > In fact we have our own implementation of basename() in our base library\n> > utils probably because of this reason. But I don't think linking against\n> > libcamera-base is really required or desired/necessary for the\n> > ipu3-pack/unpack utils so ...\n> \n> Is there a real need to compile ipu3-pack and ipu3-unpack on musl-based\n> systems ? Those are development tools that nobody has used for ages.\n\nMaybe not, but regardless they are built as part of the whole libcamera\nbuild, so ensureing we continue to compile is probably a requirement.\n\nOr we drop the utils... but I think that would be unfortunate for anyone\nwho then later jumps on to the ipu3 and wants to play around with this.\n\nBut that's exactly why dropping a const here seems ... quite minor.\n\nDo you prefer to nak this patch? Or maybe disable the build? (but let it\nbit rot?)\n\n--\nKieran\n\n\n\n> \n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > \n> > > Signed-off-by: Khem Raj <raj.khem@gmail.com>\n> > > ---\n> > >  utils/ipu3/ipu3-pack.c   | 4 ++--\n> > >  utils/ipu3/ipu3-unpack.c | 3 ++-\n> > >  2 files changed, 4 insertions(+), 3 deletions(-)\n> > > \n> > > diff --git a/utils/ipu3/ipu3-pack.c b/utils/ipu3/ipu3-pack.c\n> > > index decbfc6c..23d2db8b 100644\n> > > --- a/utils/ipu3/ipu3-pack.c\n> > > +++ b/utils/ipu3/ipu3-pack.c\n> > > @@ -8,6 +8,7 @@\n> > >  \n> > >  #include <errno.h>\n> > >  #include <fcntl.h>\n> > > +#include <libgen.h>\n> > >  #include <stdint.h>\n> > >  #include <stdio.h>\n> > >  #include <string.h>\n> > > @@ -15,9 +16,8 @@\n> > >  #include <sys/types.h>\n> > >  #include <unistd.h>\n> > >  \n> > > -static void usage(const char *argv0)\n> > > +static void usage(char *argv0)\n> > >  {\n> > > -\n> > >         printf(\"Usage: %s input-file output-file\\n\", basename(argv0));\n> > >         printf(\"Convert unpacked RAW10 Bayer data to the IPU3 packed Bayer formats\\n\");\n> > >         printf(\"If the output-file '-', output data will be written to standard output\\n\");\n> > > diff --git a/utils/ipu3/ipu3-unpack.c b/utils/ipu3/ipu3-unpack.c\n> > > index 9d2c1200..1505a970 100644\n> > > --- a/utils/ipu3/ipu3-unpack.c\n> > > +++ b/utils/ipu3/ipu3-unpack.c\n> > > @@ -8,6 +8,7 @@\n> > >  \n> > >  #include <errno.h>\n> > >  #include <fcntl.h>\n> > > +#include <libgen.h>\n> > >  #include <stdint.h>\n> > >  #include <stdio.h>\n> > >  #include <string.h>\n> > > @@ -15,7 +16,7 @@\n> > >  #include <sys/types.h>\n> > >  #include <unistd.h>\n> > >  \n> > > -static void usage(const char *argv0)\n> > > +static void usage(char *argv0)\n> > >  {\n> > >         printf(\"Usage: %s input-file output-file\\n\", basename(argv0));\n> > >         printf(\"Unpack the IPU3 raw Bayer format to 16-bit Bayer\\n\");\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C4832C3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 16 Apr 2024 16:30:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B70B96333E;\n\tTue, 16 Apr 2024 18:30:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 39B3761B75\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 16 Apr 2024 18:30:55 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 945EE512;\n\tTue, 16 Apr 2024 18:30:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"azjPbBAY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713285008;\n\tbh=X5GyZ7bhZ5SRaKPTwp1BfD2hgZGYAUirYKcRQMII6h0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=azjPbBAYO6Qe8/VD89Rncxiuc2j5LGG8vCfqfeqJU5NSbvuA8R7KTse14TsczNv72\n\tx0NaqWGY5GT/8ngOa7D3eV3d27RX7t1lamkK0fWCr4oKnweyQEJtbciwcw42sObN7c\n\tTsZUJdZiVAur8VOR1hEuDsERWqMKy6s/InkZJH5M=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240416150551.GI12561@pendragon.ideasonboard.com>","References":"<20240324183307.3833076-1-raj.khem@gmail.com>\n\t<171327953309.342316.7093064182795030881@ping.linuxembedded.co.uk>\n\t<20240416150551.GI12561@pendragon.ideasonboard.com>","Subject":"Re: [PATCH] ipu3: Use posix basename","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Khem Raj <raj.khem@gmail.com>, libcamera-devel@lists.libcamera.org","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Date":"Tue, 16 Apr 2024 17:30:51 +0100","Message-ID":"<171328505179.2468918.6974895004700654332@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29240,"web_url":"https://patchwork.libcamera.org/comment/29240/","msgid":"<20240416221630.GJ12561@pendragon.ideasonboard.com>","date":"2024-04-16T22:16:30","subject":"Re: [PATCH] ipu3: Use posix basename","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Apr 16, 2024 at 05:30:51PM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2024-04-16 16:05:51)\n> > On Tue, Apr 16, 2024 at 03:58:53PM +0100, Kieran Bingham wrote:\n> > > Quoting Khem Raj (2024-03-24 18:33:07)\n> > > > musl does not implement GNU basename extention and with latest musl\n> > > > the prototype from string.h is also removed [1] which now results in\n> > > > compile errors e.g.\n> > > > \n> > > > ../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]\n> > > > \n> > > > These utilities are using this function in usage() which is used just\n> > > > before program exit. Always use the basename APIs from libgen.h which is\n> > > > posix implementation\n> > > > \n> > > > [1] https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7\n> > > \n> > > This passes all of our compile and lint tests presently. We don't have a\n> > > Musl target in CI yet. Perhaps we should add one.\n> > > \n> > > https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1138070\n> > > \n> > > No particularly strong opinion on the patch, but it fixes an issue\n> > > for you, and is only in one of the utils, and doesn't break CI ...\n> > > \n> > > \n> > > It's a shame that this removes a const on a variable that otherwise\n> > > shouldn't be modified but ... I think we can cope with that here.\n> > > \n> > > In fact we have our own implementation of basename() in our base library\n> > > utils probably because of this reason. But I don't think linking against\n> > > libcamera-base is really required or desired/necessary for the\n> > > ipu3-pack/unpack utils so ...\n> > \n> > Is there a real need to compile ipu3-pack and ipu3-unpack on musl-based\n> > systems ? Those are development tools that nobody has used for ages.\n> \n> Maybe not, but regardless they are built as part of the whole libcamera\n> build, so ensureing we continue to compile is probably a requirement.\n> \n> Or we drop the utils... but I think that would be unfortunate for anyone\n> who then later jumps on to the ipu3 and wants to play around with this.\n> \n> But that's exactly why dropping a const here seems ... quite minor.\n> \n> Do you prefer to nak this patch? Or maybe disable the build? (but let it\n> bit rot?)\n\nWe could gate the utilities behind a meson option, but I think that's\noverkill. Gating them behind automatic detection of a compatible version\nof basename() would be a bit better, but likely overkill too. I'm OK\nwith the patch.\n\n> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > \n> > > > Signed-off-by: Khem Raj <raj.khem@gmail.com>\n> > > > ---\n> > > >  utils/ipu3/ipu3-pack.c   | 4 ++--\n> > > >  utils/ipu3/ipu3-unpack.c | 3 ++-\n> > > >  2 files changed, 4 insertions(+), 3 deletions(-)\n> > > > \n> > > > diff --git a/utils/ipu3/ipu3-pack.c b/utils/ipu3/ipu3-pack.c\n> > > > index decbfc6c..23d2db8b 100644\n> > > > --- a/utils/ipu3/ipu3-pack.c\n> > > > +++ b/utils/ipu3/ipu3-pack.c\n> > > > @@ -8,6 +8,7 @@\n> > > >  \n> > > >  #include <errno.h>\n> > > >  #include <fcntl.h>\n> > > > +#include <libgen.h>\n> > > >  #include <stdint.h>\n> > > >  #include <stdio.h>\n> > > >  #include <string.h>\n> > > > @@ -15,9 +16,8 @@\n> > > >  #include <sys/types.h>\n> > > >  #include <unistd.h>\n> > > >  \n> > > > -static void usage(const char *argv0)\n> > > > +static void usage(char *argv0)\n> > > >  {\n> > > > -\n> > > >         printf(\"Usage: %s input-file output-file\\n\", basename(argv0));\n> > > >         printf(\"Convert unpacked RAW10 Bayer data to the IPU3 packed Bayer formats\\n\");\n> > > >         printf(\"If the output-file '-', output data will be written to standard output\\n\");\n> > > > diff --git a/utils/ipu3/ipu3-unpack.c b/utils/ipu3/ipu3-unpack.c\n> > > > index 9d2c1200..1505a970 100644\n> > > > --- a/utils/ipu3/ipu3-unpack.c\n> > > > +++ b/utils/ipu3/ipu3-unpack.c\n> > > > @@ -8,6 +8,7 @@\n> > > >  \n> > > >  #include <errno.h>\n> > > >  #include <fcntl.h>\n> > > > +#include <libgen.h>\n> > > >  #include <stdint.h>\n> > > >  #include <stdio.h>\n> > > >  #include <string.h>\n> > > > @@ -15,7 +16,7 @@\n> > > >  #include <sys/types.h>\n> > > >  #include <unistd.h>\n> > > >  \n> > > > -static void usage(const char *argv0)\n> > > > +static void usage(char *argv0)\n> > > >  {\n> > > >         printf(\"Usage: %s input-file output-file\\n\", basename(argv0));\n> > > >         printf(\"Unpack the IPU3 raw Bayer format to 16-bit Bayer\\n\");","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 55560C3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 16 Apr 2024 22:16:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2BDDB63352;\n\tWed, 17 Apr 2024 00:16:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1739C63339\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 17 Apr 2024 00:16:39 +0200 (CEST)","from pendragon.ideasonboard.com (ptr-212-224-238-3.dyn.orange.be\n\t[212.224.238.3])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1ABAF55;\n\tWed, 17 Apr 2024 00:15:52 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YDlncfAu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713305752;\n\tbh=1pildKOntiXcv8yG2FQr56BBkEPZ780guJ9WFI39C74=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YDlncfAuLifScTfMIPPBs8jp1TmlldjwMoMdjRE+YWQ7IraT0Q/rqqwijIMDH/GhM\n\tMPIgAJ3pNzCSlP8XaH83/CVBJ7EJdg3gtGemEAmaE8Cxx2pIhg/0OFD20Q9W3S6suz\n\ty/biyu8lypVByAJAJkImlQ2THH6Migi9GTEIRlYo=","Date":"Wed, 17 Apr 2024 01:16:30 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Khem Raj <raj.khem@gmail.com>, libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] ipu3: Use posix basename","Message-ID":"<20240416221630.GJ12561@pendragon.ideasonboard.com>","References":"<20240324183307.3833076-1-raj.khem@gmail.com>\n\t<171327953309.342316.7093064182795030881@ping.linuxembedded.co.uk>\n\t<20240416150551.GI12561@pendragon.ideasonboard.com>\n\t<171328505179.2468918.6974895004700654332@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<171328505179.2468918.6974895004700654332@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]