Re: [raw2rgbpnm PATCH v4 04/11] Support long options and improve help text

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Tue, Mar 10, 2026 at 10:46:10AM +0200, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  raw2rgbpnm.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> index 14cfa7e7a46c..16ef64e984b9 100644
> --- a/raw2rgbpnm.c
> +++ b/raw2rgbpnm.c
> @@ -24,6 +24,7 @@
>   */
>  
>  #include <ctype.h>
> +#include <getopt.h>
>  #include <stdio.h>
>  #include <stdint.h>
>  #include <stdlib.h>
> @@ -836,6 +837,17 @@ static int parse_format(const char *p, int *w, int *h)
>  	return 0;
>  }
>  
> +static struct option options[] = {
> +	{ "algo", required_argument, NULL, 'a', },
> +	{ "brightness", required_argument, NULL, 'b', },
> +	{ "format", required_argument, NULL, 'f', },
> +	{ "help", no_argument, NULL, 'h', },
> +	{ "high-bits", no_argument, NULL, 'g', },
> +	{ "size", required_argument, NULL, 's', },
> +	{ "swap-rb", no_argument, NULL, 'w', },
> +	{ 0 },
> +};
> +
>  int main(int argc, char *argv[])
>  {
>  	FILE *f;
> @@ -849,7 +861,7 @@ int main(int argc, char *argv[])
>  	int width = -1;
>  
>  	for (;;) {
> -		int c = getopt(argc, argv, "a:b:f:ghs:w");
> +		int c = getopt_long(argc, argv, "a:b:f:ghs:w", options, NULL);
>  		if (c==-1) break;
>  		switch (c) {
>  		case 'a':
> @@ -889,14 +901,18 @@ int main(int argc, char *argv[])
>  			break;
>  		case 'h':
>  			printf("%s - Convert headerless raw image to RGB file (PNM)\n"
> -			       "Usage: %s [-h] [-w] [-s XxY] <inputfile> <outputfile>\n"
> -			       "-a <algo>     Select algorithm, use \"-a ?\" for a list\n"
> -			       "-b <bright>   Set brightness (multiplier) to output image (float, default 1.0)\n"
> -			       "-f <format>   Specify input file format format (-f ? for list, default UYVY)\n"
> -			       "-g            Use high bits for Bayer RAW 10 data\n"
> -			       "-h            Show this help\n"
> -			       "-s <XxY>      Specify image size\n"
> -			       "-w            Swap R and B channels\n", progname, argv[0]);
> +			       "Usage: %s [--algo|-a <algo>] [--brightness|-b <brightness>]\n"
> +			       "       [--format|-f <format>] [--help|-h] [--high-bits|-g] [--size|-s XxY]\n"
> +			       "       [--swap-rb|-w] <inputfile> <outputfile>\n\n"

That won't scale nicely, but I don't want to implement an options
handling framework in C right now :-)

> +			       "--algo, -a <algo>         Select algorithm, use \"-a ?\" for a list\n"
> +			       "--brightness, -b <bright> Set brightness (multiplier) to output image\n"
> +			       "                          (float, default 1.0)\n"

Do you know if this option is used by anyone ? It seems a bit out of
place. Not an issue with this patch of course.

> +			       "--format, -f <format>     Specify input file format format\n"
> +			       "                          (-f ? for list, default UYVY)\n"
> +			       "--help, -h                Show this help\n"
> +			       "--high-bits, -g           Use high bits for Bayer RAW 10 data\n"
> +			       "--size, -s <XxY>          Specify image size\n"
> +			       "--swap-rb, -w             Swap R and B channels\n", progname, argv[0]);

I'd add a line break before progname.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  			exit(0);
>  		case 's':
>  			if (parse_format(optarg, &width, &height) < 0) {

-- 
Regards,

Laurent Pinchart