[{"id":848,"web_url":"https://patchwork.libcamera.org/comment/848/","msgid":"<20190222003904.GR3485@pendragon.ideasonboard.com>","date":"2019-02-22T00:39:04","subject":"Re: [libcamera-devel] [PATCH 4/4] cam: Improve when usage\n\tinformation is printed","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Wed, Feb 20, 2019 at 03:37:36PM +0100, Niklas Söderlund wrote:\n> Running the cam tool without any options results in the tool to exit\n> with EXIT_FAILURE but no usage being printed, this is confusing. Improve\n> this by also printing the usage text.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  src/cam/main.cpp | 11 ++++++-----\n>  1 file changed, 6 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 522d2f0d3373dc25..9f4c8e26751d982c 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -41,6 +41,8 @@ void signalHandler(int signal)\n>  \n>  static int parseOptions(int argc, char *argv[])\n>  {\n> +\tint ret = 0;\n> +\n>  \tKeyValueParser formatKeyValue;\n>  \tformatKeyValue.addOption(\"width\", OptionInteger, \"Width in pixels\",\n>  \t\t\t\t ArgumentRequired);\n> @@ -67,15 +69,14 @@ static int parseOptions(int argc, char *argv[])\n>  \tparser.addOption(OptList, OptionNone, \"List all cameras\", \"list\");\n>  \n>  \toptions = parser.parse(argc, argv);\n> +\n>  \tif (!options.valid())\n> -\t\treturn -EINVAL;\n> +\t\tret = -EINVAL;\n>  \n> -\tif (argc == 1 || options.isSet(OptHelp)) {\n\nGood catch, when no options are specified options.valid() returns false,\nI had overlooked that.\n\n> +\tif (ret || options.isSet(OptHelp))\n>  \t\tparser.usage();\n> -\t\treturn 1;\n> -\t}\n>  \n> -\treturn 0;\n> +\treturn ret;\n\nHow about simplifying this to\n\n\toptions = parser.parse(argc, argv);\n\tif (!options.valid() || options.isSet(OptHelp)) {\n\t\tparser.usage();\n\t\treturn 1;\n\t}\n\n\treturn 0;\n\nWith this change,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n>  }\n>  \n>  static bool configureStreams(Camera *camera, std::vector<Stream *> &streams)","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1443B600FF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 22 Feb 2019 01:39:09 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 85E14255;\n\tFri, 22 Feb 2019 01:39:08 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1550795948;\n\tbh=8dXIQfg4wdoVRIOvKW8Fzh1ei0hVmL5pAI1PxSRda4g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vdDQ9NcVXZQ7BAQMEHPl+epb5GiZkbB8AbdiaQZTmN9IQWTS7qgLDfkq4d1asFqM6\n\tUKuyCCvKtN3gwpfd9iagDnokKJT/857IxhysIwfOm1cRlivLiZrlhYgU95yaaUihmp\n\tRWJuqfXsNS3n0iBzEMcFN1JNRoCcgT7AGiReyBQg=","Date":"Fri, 22 Feb 2019 02:39:04 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190222003904.GR3485@pendragon.ideasonboard.com>","References":"<20190220143736.529-1-niklas.soderlund@ragnatech.se>\n\t<20190220143736.529-5-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190220143736.529-5-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/4] cam: Improve when usage\n\tinformation is printed","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Fri, 22 Feb 2019 00:39:09 -0000"}},{"id":849,"web_url":"https://patchwork.libcamera.org/comment/849/","msgid":"<20190222013836.GK11484@bigcity.dyn.berto.se>","date":"2019-02-22T01:38:36","subject":"Re: [libcamera-devel] [PATCH 4/4] cam: Improve when usage\n\tinformation is printed","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2019-02-22 02:39:04 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Wed, Feb 20, 2019 at 03:37:36PM +0100, Niklas Söderlund wrote:\n> > Running the cam tool without any options results in the tool to exit\n> > with EXIT_FAILURE but no usage being printed, this is confusing. Improve\n> > this by also printing the usage text.\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  src/cam/main.cpp | 11 ++++++-----\n> >  1 file changed, 6 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index 522d2f0d3373dc25..9f4c8e26751d982c 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -41,6 +41,8 @@ void signalHandler(int signal)\n> >  \n> >  static int parseOptions(int argc, char *argv[])\n> >  {\n> > +\tint ret = 0;\n> > +\n> >  \tKeyValueParser formatKeyValue;\n> >  \tformatKeyValue.addOption(\"width\", OptionInteger, \"Width in pixels\",\n> >  \t\t\t\t ArgumentRequired);\n> > @@ -67,15 +69,14 @@ static int parseOptions(int argc, char *argv[])\n> >  \tparser.addOption(OptList, OptionNone, \"List all cameras\", \"list\");\n> >  \n> >  \toptions = parser.parse(argc, argv);\n> > +\n> >  \tif (!options.valid())\n> > -\t\treturn -EINVAL;\n> > +\t\tret = -EINVAL;\n> >  \n> > -\tif (argc == 1 || options.isSet(OptHelp)) {\n> \n> Good catch, when no options are specified options.valid() returns false,\n> I had overlooked that.\n> \n> > +\tif (ret || options.isSet(OptHelp))\n> >  \t\tparser.usage();\n> > -\t\treturn 1;\n> > -\t}\n> >  \n> > -\treturn 0;\n> > +\treturn ret;\n> \n> How about simplifying this to\n> \n> \toptions = parser.parse(argc, argv);\n> \tif (!options.valid() || options.isSet(OptHelp)) {\n> \t\tparser.usage();\n> \t\treturn 1;\n> \t}\n\nI considered this when creating the patch and decided I did not like the \nend result. I like the distinction that if something goes wrong a error \ncode (which current design needs to be negative) is returned.\n\nWith this change 'cam --help' would result in the return value from the \ntool would be EXIT_FAILURE. If we all are OK with this behavior for \n--help I would be open to take your suggestion and make parseOptions() \nreturn a bool instead of an int. What do you all think?\n\n> \n> \treturn 0;\n> \n> With this change,\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> >  }\n> >  \n> >  static bool configureStreams(Camera *camera, std::vector<Stream *> &streams)\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6127B600FF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 22 Feb 2019 02:38:38 +0100 (CET)","by mail-lj1-x241.google.com with SMTP id z7so190489lji.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 21 Feb 2019 17:38:38 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tk124sm20586lfg.95.2019.02.21.17.38.36\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 21 Feb 2019 17:38:36 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=0TFE7PIKJoRQF8toVvvj8RuYoDlvkNl5kRlZyuxb9bw=;\n\tb=IsrxMBQ68fIHc+x7GI+rhgC5KbHJChlsBHdlQMy3RkJebUGAbwXJKSasrSHMMPcR2F\n\tocMjmJRQCexMD1ie/Gin0/GOWmZXRMXi7f1rkF4smaH5r3Tm/h02Bu0VSe7BtJ//jeas\n\tQPBq1QKXMSv3x6175nF7oGRlLy6gCsLmQ8leZsia//Ui1GvHiSiCRK9MqKYx/2AbUF0d\n\tR8qVXSJLzSFC8DT3oWUVM47bVkr2FXE9z67eWPRlrDo4L3NhHgAWT2p6IWgyXgMrbndq\n\tqwJHEzXEgDKS9GngzhKsrCloTPw9yJJdU4FcBKuyWHFkM5Eed0Dbyi9Dr/ABzHc1XT7l\n\tQSsQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=0TFE7PIKJoRQF8toVvvj8RuYoDlvkNl5kRlZyuxb9bw=;\n\tb=OiPKPWOdtcq8Bl/pn+NWCSHd79O+m029AMAHmRntdlZHnPBo6OdHVd3Qn50ch7qMZS\n\tTv8LJT6DV0Lp7+KaiZyCWGOXkcMnMvpuFEyH5eTC/HQCBPxvP8zwUmUACkZ8+O4k4nmS\n\te0T+STNAgWEHw54xQeYJhoWPePalHTe12EWm7dw5r+D0mcBeK+RcI6fEOvSFc5pwKsDM\n\tUPRJb/bbn0ZQ2ENpDPhCdh5+6j2KTHIKg19JsImCDXYJSQpaqWgGidWTcY9ymvirprac\n\t1DvjFihDOxfbPp5qANt3G1xh6jVj2pjcvq2D6ZEreQOyv34//+oL+lXpibXtvI9ADIop\n\txRrw==","X-Gm-Message-State":"AHQUAuaNsQfH7v6kqVTrmLhYJizHN88GzaYsRbaf0MT7cgKWWY224Wx+\n\tnT7ih0/piwzAsjYhvpZMHHDc+zVcZ+4=","X-Google-Smtp-Source":"AHgI3IYaN+5GyZ53jaiXu1c4ec0dXINWVPYsMxlZXihP90poBp/T9VhKvRUui2SZnKmh6zJbmpRyvw==","X-Received":"by 2002:a2e:5d59:: with SMTP id r86mr681355ljb.158.1550799517622;\n\tThu, 21 Feb 2019 17:38:37 -0800 (PST)","Date":"Fri, 22 Feb 2019 02:38:36 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190222013836.GK11484@bigcity.dyn.berto.se>","References":"<20190220143736.529-1-niklas.soderlund@ragnatech.se>\n\t<20190220143736.529-5-niklas.soderlund@ragnatech.se>\n\t<20190222003904.GR3485@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190222003904.GR3485@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/4] cam: Improve when usage\n\tinformation is printed","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Fri, 22 Feb 2019 01:38:38 -0000"}},{"id":852,"web_url":"https://patchwork.libcamera.org/comment/852/","msgid":"<20190222095734.GB3522@pendragon.ideasonboard.com>","date":"2019-02-22T09:57:34","subject":"Re: [libcamera-devel] [PATCH 4/4] cam: Improve when usage\n\tinformation is printed","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Fri, Feb 22, 2019 at 02:38:36AM +0100, Niklas Söderlund wrote:\n> On 2019-02-22 02:39:04 +0200, Laurent Pinchart wrote:\n> > On Wed, Feb 20, 2019 at 03:37:36PM +0100, Niklas Söderlund wrote:\n> >> Running the cam tool without any options results in the tool to exit\n> >> with EXIT_FAILURE but no usage being printed, this is confusing. Improve\n> >> this by also printing the usage text.\n> >> \n> >> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> >> ---\n> >>  src/cam/main.cpp | 11 ++++++-----\n> >>  1 file changed, 6 insertions(+), 5 deletions(-)\n> >> \n> >> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> >> index 522d2f0d3373dc25..9f4c8e26751d982c 100644\n> >> --- a/src/cam/main.cpp\n> >> +++ b/src/cam/main.cpp\n> >> @@ -41,6 +41,8 @@ void signalHandler(int signal)\n> >>  \n> >>  static int parseOptions(int argc, char *argv[])\n> >>  {\n> >> +\tint ret = 0;\n> >> +\n> >>  \tKeyValueParser formatKeyValue;\n> >>  \tformatKeyValue.addOption(\"width\", OptionInteger, \"Width in pixels\",\n> >>  \t\t\t\t ArgumentRequired);\n> >> @@ -67,15 +69,14 @@ static int parseOptions(int argc, char *argv[])\n> >>  \tparser.addOption(OptList, OptionNone, \"List all cameras\", \"list\");\n> >>  \n> >>  \toptions = parser.parse(argc, argv);\n> >> +\n> >>  \tif (!options.valid())\n> >> -\t\treturn -EINVAL;\n> >> +\t\tret = -EINVAL;\n> >>  \n> >> -\tif (argc == 1 || options.isSet(OptHelp)) {\n> > \n> > Good catch, when no options are specified options.valid() returns false,\n> > I had overlooked that.\n> > \n> >> +\tif (ret || options.isSet(OptHelp))\n> >>  \t\tparser.usage();\n> >> -\t\treturn 1;\n> >> -\t}\n> >>  \n> >> -\treturn 0;\n> >> +\treturn ret;\n> > \n> > How about simplifying this to\n> > \n> > \toptions = parser.parse(argc, argv);\n> > \tif (!options.valid() || options.isSet(OptHelp)) {\n> > \t\tparser.usage();\n> > \t\treturn 1;\n> > \t}\n> \n> I considered this when creating the patch and decided I did not like the \n> end result. I like the distinction that if something goes wrong a error \n> code (which current design needs to be negative) is returned.\n> \n> With this change 'cam --help' would result in the return value from the \n> tool would be EXIT_FAILURE. If we all are OK with this behavior for \n> --help I would be open to take your suggestion and make parseOptions() \n> return a bool instead of an int. What do you all think?\n\nThe help option usually results in no other option being processed and\nthe application returning immediately. I think we should preserve that\nbehaviour. We may not want to return EXIT_FAILURE in that case though.\n\n \toptions = parser.parse(argc, argv);\n \tif (!options.valid() || options.isSet(OptHelp)) {\n \t\tparser.usage();\n \t\treturn !options.valid() ? -EINVAL : -EINTR;\n\t}\n\nAnd in the caller\n\n\treturn ret == -EINTR ? 0 : EXIT_FAILURE;\n\n?\n\n> > \n> > \treturn 0;\n> > \n> > With this change,\n> > \n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> >>  }\n> >>  \n> >>  static bool configureStreams(Camera *camera, std::vector<Stream *> &streams)","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A795E600FD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 22 Feb 2019 10:57:38 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 226D1255;\n\tFri, 22 Feb 2019 10:57:38 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1550829458;\n\tbh=a+Ika1hwiJ+Kk7IRF/7hoxAJhFGG93KzNO2bspnaTlg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=P7f0TNfEpI8V5V6qHf/TArDOVKVJqZtv0jjM4rFaPdsIZnk6rY++NnAqF/R54jOEq\n\tI+8qJyPMM0zz/5GE10DT4aQ9bthaAMmXAZZWYgSx/BX5SNFt24MEECQCs0SwbsYNRf\n\tn7dyuFrXMpQ8FmxLwJWBqw/eVOtbWs8qevLFG1n8=","Date":"Fri, 22 Feb 2019 11:57:34 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190222095734.GB3522@pendragon.ideasonboard.com>","References":"<20190220143736.529-1-niklas.soderlund@ragnatech.se>\n\t<20190220143736.529-5-niklas.soderlund@ragnatech.se>\n\t<20190222003904.GR3485@pendragon.ideasonboard.com>\n\t<20190222013836.GK11484@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190222013836.GK11484@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/4] cam: Improve when usage\n\tinformation is printed","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Fri, 22 Feb 2019 09:57:38 -0000"}},{"id":868,"web_url":"https://patchwork.libcamera.org/comment/868/","msgid":"<20190224170726.GB899@bigcity.dyn.berto.se>","date":"2019-02-24T17:07:26","subject":"Re: [libcamera-devel] [PATCH 4/4] cam: Improve when usage\n\tinformation is printed","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2019-02-22 11:57:34 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Fri, Feb 22, 2019 at 02:38:36AM +0100, Niklas Söderlund wrote:\n> > On 2019-02-22 02:39:04 +0200, Laurent Pinchart wrote:\n> > > On Wed, Feb 20, 2019 at 03:37:36PM +0100, Niklas Söderlund wrote:\n> > >> Running the cam tool without any options results in the tool to exit\n> > >> with EXIT_FAILURE but no usage being printed, this is confusing. Improve\n> > >> this by also printing the usage text.\n> > >> \n> > >> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > >> ---\n> > >>  src/cam/main.cpp | 11 ++++++-----\n> > >>  1 file changed, 6 insertions(+), 5 deletions(-)\n> > >> \n> > >> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > >> index 522d2f0d3373dc25..9f4c8e26751d982c 100644\n> > >> --- a/src/cam/main.cpp\n> > >> +++ b/src/cam/main.cpp\n> > >> @@ -41,6 +41,8 @@ void signalHandler(int signal)\n> > >>  \n> > >>  static int parseOptions(int argc, char *argv[])\n> > >>  {\n> > >> +\tint ret = 0;\n> > >> +\n> > >>  \tKeyValueParser formatKeyValue;\n> > >>  \tformatKeyValue.addOption(\"width\", OptionInteger, \"Width in pixels\",\n> > >>  \t\t\t\t ArgumentRequired);\n> > >> @@ -67,15 +69,14 @@ static int parseOptions(int argc, char *argv[])\n> > >>  \tparser.addOption(OptList, OptionNone, \"List all cameras\", \"list\");\n> > >>  \n> > >>  \toptions = parser.parse(argc, argv);\n> > >> +\n> > >>  \tif (!options.valid())\n> > >> -\t\treturn -EINVAL;\n> > >> +\t\tret = -EINVAL;\n> > >>  \n> > >> -\tif (argc == 1 || options.isSet(OptHelp)) {\n> > > \n> > > Good catch, when no options are specified options.valid() returns false,\n> > > I had overlooked that.\n> > > \n> > >> +\tif (ret || options.isSet(OptHelp))\n> > >>  \t\tparser.usage();\n> > >> -\t\treturn 1;\n> > >> -\t}\n> > >>  \n> > >> -\treturn 0;\n> > >> +\treturn ret;\n> > > \n> > > How about simplifying this to\n> > > \n> > > \toptions = parser.parse(argc, argv);\n> > > \tif (!options.valid() || options.isSet(OptHelp)) {\n> > > \t\tparser.usage();\n> > > \t\treturn 1;\n> > > \t}\n> > \n> > I considered this when creating the patch and decided I did not like the \n> > end result. I like the distinction that if something goes wrong a error \n> > code (which current design needs to be negative) is returned.\n> > \n> > With this change 'cam --help' would result in the return value from the \n> > tool would be EXIT_FAILURE. If we all are OK with this behavior for \n> > --help I would be open to take your suggestion and make parseOptions() \n> > return a bool instead of an int. What do you all think?\n> \n> The help option usually results in no other option being processed and\n> the application returning immediately. I think we should preserve that\n> behaviour. We may not want to return EXIT_FAILURE in that case though.\n> \n>  \toptions = parser.parse(argc, argv);\n>  \tif (!options.valid() || options.isSet(OptHelp)) {\n>  \t\tparser.usage();\n>  \t\treturn !options.valid() ? -EINVAL : -EINTR;\n> \t}\n> \n> And in the caller\n> \n> \treturn ret == -EINTR ? 0 : EXIT_FAILURE;\n> \n> ?\n\nI like this and will make it so.\n\n> \n> > > \n> > > \treturn 0;\n> > > \n> > > With this change,\n> > > \n> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > \n> > >>  }\n> > >>  \n> > >>  static bool configureStreams(Camera *camera, std::vector<Stream *> &streams)\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A3F2601E2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 24 Feb 2019 18:07:29 +0100 (CET)","by mail-lj1-x244.google.com with SMTP id a17so5415531ljd.4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 24 Feb 2019 09:07:29 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\ty1sm260730ljj.13.2019.02.24.09.07.26\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 24 Feb 2019 09:07:26 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=TtIYKKegevWccbS1id4Xs3z2MBbG9/IAeyIiWDofdiY=;\n\tb=dnXIqpbz63/+dG/boL7KkelJiC7J0C87yx3GkU47QmL3s6TrBpNyG73lqrFDKFbGJO\n\tHt5Tj8QP/hliwPUeC2dslGkUr0D9A2PUXHK8+kGlV2xrqTP0DHNZHtt8Tx+H5aYa8iBx\n\tKv1x2wF2o4dmXApWngSp0UHmkjNV0vznhFEw02bv62JN+ZthhvVuCzmpi2anBhY+EPgL\n\tHsNeivi+facXj26d80ToHn1wUt8Ve2PFzLhLUJyH8jTEFBJGZDv79+ooHLvtp5CdqyPV\n\tz35Kcbjc/DhBuwZQ9Uk/tY4MRIx6bIrh3I5FiotQGa4o1AZJb9P+FUqhm0Jmrq0ZUuQc\n\tFM+A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=TtIYKKegevWccbS1id4Xs3z2MBbG9/IAeyIiWDofdiY=;\n\tb=Uc5v2ydpBXZY6B34vVrQ7CtLyDzTMGFmaaG3v3WeNqAI3rOoTPJapbhTlM8KW45hc0\n\tRd/mEKi/StoxtfaxMIot6/mlV16a4byBNIBBKZIAQ4zbo1PQd4mVeSUDF2Qc1pgZrD/v\n\twSr9e/vC4GGAqReyCzE9eLgHUnwML5p5zcDXJX4jsK2iYA+R0G406B+4scko9GrDR7as\n\tCfho3DOWm1bhMhyqNMuoXIGD9FVVGEcCplCxWcJ/iqMvYOc/LxD/qHbPdTKCrTBLBDpX\n\tMyLV4wRJ9pBNpqCMM6Sdt0HnCUrfQNqhLN0G4cwS08M3AhDiaR0HHzJ1qIUqVNFpkEVx\n\tKvlw==","X-Gm-Message-State":"AHQUAuZmliwhV06ZVvN0zp8vespChtqamigQF6ctdvZa10P58vD2XmRY\n\tgYpiKi03D28N1dDIP9f1/RY439oVbTY=","X-Google-Smtp-Source":"AHgI3IbEU2GWR1AjsONpwzsg0Xn6MWXBWvbcpYzmAmBrixJkpP0vhZ9D249DFWhnzLupEjiJApIhqg==","X-Received":"by 2002:a2e:7806:: with SMTP id t6mr6921973ljc.188.1551028048350;\n\tSun, 24 Feb 2019 09:07:28 -0800 (PST)","Date":"Sun, 24 Feb 2019 18:07:26 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190224170726.GB899@bigcity.dyn.berto.se>","References":"<20190220143736.529-1-niklas.soderlund@ragnatech.se>\n\t<20190220143736.529-5-niklas.soderlund@ragnatech.se>\n\t<20190222003904.GR3485@pendragon.ideasonboard.com>\n\t<20190222013836.GK11484@bigcity.dyn.berto.se>\n\t<20190222095734.GB3522@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190222095734.GB3522@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/4] cam: Improve when usage\n\tinformation is printed","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Sun, 24 Feb 2019 17:07:29 -0000"}}]