[PATCH 0/7] raw2rgbpnm cleanups and a fix

[PATCH 0/7] raw2rgbpnm cleanups and a fix

[Sakari Ailus]

Hi folks,

Here are a few cleanups and fixes found in Laurent's review.

Sakari Ailus (7):
  Reorder headers alphabetically
  Check we have a supported format first before allocating memory
  Add quotes for "-f help" in help text and do a line wrap
  Constify progname
  More header sorting
  Use "stride" in a stride-related error message instead of an internal
    name
  Fix file size check

 raw2rgbpnm.c | 47 +++++++++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

-- 
2.47.3

[PATCH 1/7] Reorder headers alphabetically

[Sakari Ailus]

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 raw2rgbpnm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
index 24a88feddf00..121f0e0272e4 100644
--- a/raw2rgbpnm.c
+++ b/raw2rgbpnm.c
@@ -26,8 +26,8 @@
 #include <ctype.h>
 #include <getopt.h>
 #include <limits.h>
-#include <stdio.h>
 #include <stdint.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
-- 
2.47.3

Re: [PATCH 1/7] Reorder headers alphabetically

[Laurent Pinchart]

On Tue, Mar 17, 2026 at 11:57:56AM +0200, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  raw2rgbpnm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> index 24a88feddf00..121f0e0272e4 100644
> --- a/raw2rgbpnm.c
> +++ b/raw2rgbpnm.c
> @@ -26,8 +26,8 @@
>  #include <ctype.h>
>  #include <getopt.h>
>  #include <limits.h>
> -#include <stdio.h>
>  #include <stdint.h>
> +#include <stdio.h>
>  #include <stdlib.h>
>  #include <unistd.h>
>  #include <string.h>

How about you select the whole block and use the sort function of your
favourite text editor ?

-- 
Regards,

Laurent Pinchart

[PATCH 2/7] Check we have a supported format first before allocating memory

[Sakari Ailus]

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 raw2rgbpnm.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
index 121f0e0272e4..8aa7258218ad 100644
--- a/raw2rgbpnm.c
+++ b/raw2rgbpnm.c
@@ -370,20 +370,7 @@ static void raw_to_rgb(const struct format_info *info,
 		src_width &= ~3;
 
 		const struct format_info *old_info = info;
-		unsigned int new_stride = src_width * 2;
-
-		tmp_src = malloc(new_stride * src_height);
-		if (!tmp_src)
-			error("can't allocate memory for the temporary buffer");
-
-		for (src_y = 0; src_y < src_height; src_y++)
-			for (src_x = 0; src_x < src_width; src_x++)
-				raw_put(16, tmp_src, new_stride, src_x, src_y,
-					raw_get(info->bpp, src, src_stride,
-						src_x, src_y));
-
-		src_stride = new_stride;
-		src = tmp_src;
+		unsigned int unpacked_stride = src_width * 2;
 
 		for (unsigned int i = 0; i < SIZE(v4l2_pix_fmt_str); i++) {
 			if (v4l2_pix_fmt_str[i].fmt == info->compat_fmt) {
@@ -395,6 +382,19 @@ static void raw_to_rgb(const struct format_info *info,
 		if (info == old_info)
 			error("no supported format found for %s",
 			      old_info->name);
+
+		tmp_src = malloc(unpacked_stride * src_height);
+		if (!tmp_src)
+			error("can't allocate memory for the temporary buffer");
+
+		for (src_y = 0; src_y < src_height; src_y++)
+			for (src_x = 0; src_x < src_width; src_x++)
+				raw_put(16, tmp_src, unpacked_stride, src_x, src_y,
+					raw_get(info->bpp, src, src_stride,
+						src_x, src_y));
+
+		src_stride = unpacked_stride;
+		src = tmp_src;
 	}
 	}
 
-- 
2.47.3

Re: [PATCH 2/7] Check we have a supported format first before allocating memory

[Laurent Pinchart]

On Tue, Mar 17, 2026 at 11:57:57AM +0200, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  raw2rgbpnm.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> index 121f0e0272e4..8aa7258218ad 100644
> --- a/raw2rgbpnm.c
> +++ b/raw2rgbpnm.c
> @@ -370,20 +370,7 @@ static void raw_to_rgb(const struct format_info *info,
>  		src_width &= ~3;
>  
>  		const struct format_info *old_info = info;
> -		unsigned int new_stride = src_width * 2;
> -
> -		tmp_src = malloc(new_stride * src_height);
> -		if (!tmp_src)
> -			error("can't allocate memory for the temporary buffer");
> -
> -		for (src_y = 0; src_y < src_height; src_y++)
> -			for (src_x = 0; src_x < src_width; src_x++)
> -				raw_put(16, tmp_src, new_stride, src_x, src_y,
> -					raw_get(info->bpp, src, src_stride,
> -						src_x, src_y));
> -
> -		src_stride = new_stride;
> -		src = tmp_src;
> +		unsigned int unpacked_stride = src_width * 2;
>  
>  		for (unsigned int i = 0; i < SIZE(v4l2_pix_fmt_str); i++) {
>  			if (v4l2_pix_fmt_str[i].fmt == info->compat_fmt) {
> @@ -395,6 +382,19 @@ static void raw_to_rgb(const struct format_info *info,
>  		if (info == old_info)
>  			error("no supported format found for %s",
>  			      old_info->name);
> +
> +		tmp_src = malloc(unpacked_stride * src_height);
> +		if (!tmp_src)
> +			error("can't allocate memory for the temporary buffer");
> +
> +		for (src_y = 0; src_y < src_height; src_y++)
> +			for (src_x = 0; src_x < src_width; src_x++)
> +				raw_put(16, tmp_src, unpacked_stride, src_x, src_y,
> +					raw_get(info->bpp, src, src_stride,

info now points to the format_info for the compat format. I think you
should now use old_info->bpp here.

> +						src_x, src_y));
> +
> +		src_stride = unpacked_stride;
> +		src = tmp_src;
>  	}
>  	}
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/7] Check we have a supported format first before allocating memory

[Sakari Ailus]

On Tue, Mar 17, 2026 at 12:54:00PM +0200, Laurent Pinchart wrote:
> On Tue, Mar 17, 2026 at 11:57:57AM +0200, Sakari Ailus wrote:
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  raw2rgbpnm.c | 28 ++++++++++++++--------------
> >  1 file changed, 14 insertions(+), 14 deletions(-)
> > 
> > diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> > index 121f0e0272e4..8aa7258218ad 100644
> > --- a/raw2rgbpnm.c
> > +++ b/raw2rgbpnm.c
> > @@ -370,20 +370,7 @@ static void raw_to_rgb(const struct format_info *info,
> >  		src_width &= ~3;
> >  
> >  		const struct format_info *old_info = info;
> > -		unsigned int new_stride = src_width * 2;
> > -
> > -		tmp_src = malloc(new_stride * src_height);
> > -		if (!tmp_src)
> > -			error("can't allocate memory for the temporary buffer");
> > -
> > -		for (src_y = 0; src_y < src_height; src_y++)
> > -			for (src_x = 0; src_x < src_width; src_x++)
> > -				raw_put(16, tmp_src, new_stride, src_x, src_y,
> > -					raw_get(info->bpp, src, src_stride,
> > -						src_x, src_y));
> > -
> > -		src_stride = new_stride;
> > -		src = tmp_src;
> > +		unsigned int unpacked_stride = src_width * 2;
> >  
> >  		for (unsigned int i = 0; i < SIZE(v4l2_pix_fmt_str); i++) {
> >  			if (v4l2_pix_fmt_str[i].fmt == info->compat_fmt) {
> > @@ -395,6 +382,19 @@ static void raw_to_rgb(const struct format_info *info,
> >  		if (info == old_info)
> >  			error("no supported format found for %s",
> >  			      old_info->name);
> > +
> > +		tmp_src = malloc(unpacked_stride * src_height);
> > +		if (!tmp_src)
> > +			error("can't allocate memory for the temporary buffer");
> > +
> > +		for (src_y = 0; src_y < src_height; src_y++)
> > +			for (src_x = 0; src_x < src_width; src_x++)
> > +				raw_put(16, tmp_src, unpacked_stride, src_x, src_y,
> > +					raw_get(info->bpp, src, src_stride,
> 
> info now points to the format_info for the compat format. I think you
> should now use old_info->bpp here.

Yes, makes sense.

> 
> > +						src_x, src_y));
> > +
> > +		src_stride = unpacked_stride;
> > +		src = tmp_src;
> >  	}
> >  	}
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Sakari Ailus

[PATCH 3/7] Add quotes for "-f help" in help text and do a line wrap

[Sakari Ailus]

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 raw2rgbpnm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
index 8aa7258218ad..e8d72a68be30 100644
--- a/raw2rgbpnm.c
+++ b/raw2rgbpnm.c
@@ -929,11 +929,12 @@ int main(int argc, char *argv[])
 			       "--brightness, -b <bright> Set brightness (multiplier) to output image\n"
 			       "                          (float, default 1.0)\n"
 			       "--format, -f <format>     Specify input file format format\n"
-			       "                          (-f help for list, default UYVY)\n"
+			       "                          (\"-f help\" 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]);
+			       "--swap-rb, -w             Swap R and B channels\n",
+			       progname, argv[0]);
 			exit(0);
 		case 's':
 			if (parse_format(optarg, &width, &height) < 0) {
-- 
2.47.3

Re: [PATCH 3/7] Add quotes for "-f help" in help text and do a line wrap

[Laurent Pinchart]

On Tue, Mar 17, 2026 at 11:57:58AM +0200, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

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

> ---
>  raw2rgbpnm.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> index 8aa7258218ad..e8d72a68be30 100644
> --- a/raw2rgbpnm.c
> +++ b/raw2rgbpnm.c
> @@ -929,11 +929,12 @@ int main(int argc, char *argv[])
>  			       "--brightness, -b <bright> Set brightness (multiplier) to output image\n"
>  			       "                          (float, default 1.0)\n"
>  			       "--format, -f <format>     Specify input file format format\n"
> -			       "                          (-f help for list, default UYVY)\n"
> +			       "                          (\"-f help\" 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]);
> +			       "--swap-rb, -w             Swap R and B channels\n",
> +			       progname, argv[0]);
>  			exit(0);
>  		case 's':
>  			if (parse_format(optarg, &width, &height) < 0) {

-- 
Regards,

Laurent Pinchart

[PATCH 4/7] Constify progname

[Sakari Ailus]

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 raw2rgbpnm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
index e8d72a68be30..215decd65d93 100644
--- a/raw2rgbpnm.c
+++ b/raw2rgbpnm.c
@@ -43,7 +43,7 @@
 #define MAX(a,b)	((a)>(b)?(a):(b))
 #define MIN(a,b)	((a)<(b)?(a):(b))
 
-char *progname = "raw2rgbpnm";
+const char *progname = "raw2rgbpnm";
 
 static int swaprb = 0;
 static int highbits = 0;			/* Bayer RAW10 formats use high bits for data */
-- 
2.47.3

Re: [PATCH 4/7] Constify progname

[Laurent Pinchart]

On Tue, Mar 17, 2026 at 11:57:59AM +0200, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

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

> ---
>  raw2rgbpnm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> index e8d72a68be30..215decd65d93 100644
> --- a/raw2rgbpnm.c
> +++ b/raw2rgbpnm.c
> @@ -43,7 +43,7 @@
>  #define MAX(a,b)	((a)>(b)?(a):(b))
>  #define MIN(a,b)	((a)<(b)?(a):(b))
>  
> -char *progname = "raw2rgbpnm";
> +const char *progname = "raw2rgbpnm";
>  
>  static int swaprb = 0;
>  static int highbits = 0;			/* Bayer RAW10 formats use high bits for data */

-- 
Regards,

Laurent Pinchart

[PATCH 5/7] More header sorting

[Sakari Ailus]

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 raw2rgbpnm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
index 215decd65d93..ef12be820be6 100644
--- a/raw2rgbpnm.c
+++ b/raw2rgbpnm.c
@@ -29,10 +29,12 @@
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <unistd.h>
 #include <string.h>
-#include <sys/types.h>
+#include <unistd.h>
+
 #include <linux/videodev2.h>
+#include <sys/types.h>
+
 #include "utils.h"
 #include "raw_to_rgb.h"
 #include "yuv_to_rgb.h"
-- 
2.47.3

Re: [PATCH 5/7] More header sorting

[Laurent Pinchart]

On Tue, Mar 17, 2026 at 11:58:00AM +0200, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Merged that with 1/7.

> ---
>  raw2rgbpnm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> index 215decd65d93..ef12be820be6 100644
> --- a/raw2rgbpnm.c
> +++ b/raw2rgbpnm.c
> @@ -29,10 +29,12 @@
>  #include <stdint.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <unistd.h>
>  #include <string.h>
> -#include <sys/types.h>
> +#include <unistd.h>
> +
>  #include <linux/videodev2.h>
> +#include <sys/types.h>
> +
>  #include "utils.h"
>  #include "raw_to_rgb.h"
>  #include "yuv_to_rgb.h"

-- 
Regards,

Laurent Pinchart

[PATCH 6/7] Use "stride" in a stride-related error message instead of an internal name

[Sakari Ailus]

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 raw2rgbpnm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
index ef12be820be6..3b92c6ba519b 100644
--- a/raw2rgbpnm.c
+++ b/raw2rgbpnm.c
@@ -177,7 +177,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
 	line_length = line_length ?: (*width * bpp + 7) / 8;
 	if (!line_length || UINT_MAX / line_length < *height ||
 	    line_length < *width * bpp / 8)
-		error("line_length %u is bad", line_length);
+		error("stride %u is bad", line_length);
 	if (file_size > UINT_MAX)
 		error("too large file");
 	if ((unsigned int)file_size < line_length * *height)
-- 
2.47.3

Re: [PATCH 6/7] Use "stride" in a stride-related error message instead of an internal name

[Laurent Pinchart]

On Tue, Mar 17, 2026 at 11:58:01AM +0200, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

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

> ---
>  raw2rgbpnm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> index ef12be820be6..3b92c6ba519b 100644
> --- a/raw2rgbpnm.c
> +++ b/raw2rgbpnm.c
> @@ -177,7 +177,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
>  	line_length = line_length ?: (*width * bpp + 7) / 8;
>  	if (!line_length || UINT_MAX / line_length < *height ||
>  	    line_length < *width * bpp / 8)
> -		error("line_length %u is bad", line_length);
> +		error("stride %u is bad", line_length);
>  	if (file_size > UINT_MAX)
>  		error("too large file");
>  	if ((unsigned int)file_size < line_length * *height)

-- 
Regards,

Laurent Pinchart

[PATCH 7/7] Fix file size check

[Sakari Ailus]

The file size was unintentionally multiplied by 8 and that's not right
anymore due to changes elsewhere.

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Fixes: 578f7012a851 ("Improve input validation")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 raw2rgbpnm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
index 3b92c6ba519b..49b8071b299e 100644
--- a/raw2rgbpnm.c
+++ b/raw2rgbpnm.c
@@ -187,7 +187,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
 	if (file_size % *height == 0) {
 		padding = file_size / *height - line_length;
 		printf("%u padding bytes detected at end of line\n", padding);
-	} else if ((file_size * 8) % (line_length * *height) != 0) {
+	} else if (file_size % (line_length * *height) != 0) {
 		printf("warning: input size not multiple of frame size\n");
 	}
 
-- 
2.47.3

Re: [PATCH 7/7] Fix file size check

[Laurent Pinchart]

On Tue, Mar 17, 2026 at 11:58:02AM +0200, Sakari Ailus wrote:
> The file size was unintentionally multiplied by 8 and that's not right
> anymore due to changes elsewhere.
> 
> Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Fixes: 578f7012a851 ("Improve input validation")
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

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

> ---
>  raw2rgbpnm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> index 3b92c6ba519b..49b8071b299e 100644
> --- a/raw2rgbpnm.c
> +++ b/raw2rgbpnm.c
> @@ -187,7 +187,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
>  	if (file_size % *height == 0) {
>  		padding = file_size / *height - line_length;
>  		printf("%u padding bytes detected at end of line\n", padding);
> -	} else if ((file_size * 8) % (line_length * *height) != 0) {
> +	} else if (file_size % (line_length * *height) != 0) {
>  		printf("warning: input size not multiple of frame size\n");
>  	}
>  

-- 
Regards,

Laurent Pinchart