Linux Media Controller development
 help / color / mirror / Atom feed
* [raw2rgbpnm] [PATCH] Fix sign compare error when checking file size
@ 2026-11-11  1:25 Paul Elder
  2026-11-11  8:50 ` Laurent Pinchart
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Elder @ 2026-11-11  1:25 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, sakari.ailus, Paul Elder

Fix the sign compare error when checking file size by casting the file
size returned by ftell to unsigned int, as it is a signed long being
compared against UINT_MAX.

This fixes the following compiler error:

FAILED: [code=1] raw2rgbpnm.p/raw2rgbpnm.c.o
/home/autobuild/autobuild/instance-5/output-1/host/bin/arm-buildroot-linux-gnueabi-gcc -Iraw2rgbpnm.p -I. -I.. -I../include -fdiagnostics-color=always -Wall -Winvalid-pch -Wextra -Werror -O3 -Wno-missing-field-initializers -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -g0 -D_FORTIFY_SOURCE=1 -MD -MQ raw2rgbpnm.p/raw2rgbpnm.c.o -MF raw2rgbpnm.p/raw2rgbpnm.c.o.d -o raw2rgbpnm.p/raw2rgbpnm.c.o -c ../raw2rgbpnm.c
../raw2rgbpnm.c: In function 'read_raw_data':
../raw2rgbpnm.c:173:36: error: comparison of integer expressions of different signedness: 'unsigned int' and 'long int' [-Werror=sign-compare]
  173 |                 if (UINT_MAX / bpp < file_size)
      |                                    ^
../raw2rgbpnm.c:185:23: error: comparison of integer expressions of different signedness: 'long int' and 'unsigned int' [-Werror=sign-compare]
  185 |         if (file_size > UINT_MAX)
      |                       ^

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
I've been getting build failure reports from buildroot for this.
https://autobuild.buildroot.org/results/f1f070bc5ac72d8b0f2ed403273635b7459a9ae5/
---
 raw2rgbpnm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
index 0c71be321746..39b186473624 100644
--- a/raw2rgbpnm.c
+++ b/raw2rgbpnm.c
@@ -170,7 +170,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
 	}
 
 	if (!*height && line_length) {
-		if (UINT_MAX / bpp < file_size)
+		if (UINT_MAX / bpp < (unsigned int)file_size)
 			error("line length too large");
 		*height = file_size / line_length;
 		*width = line_length * 8 / bpp;
@@ -182,7 +182,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
 	if (!line_length || UINT_MAX / line_length < *height ||
 	    line_length < *width * bpp / 8)
 		error("stride %u is bad", line_length);
-	if (file_size > UINT_MAX)
+	if ((unsigned int)file_size > UINT_MAX)
 		error("too large file");
 	if ((unsigned int)file_size < line_length * *height)
 		error("out of input data");
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [raw2rgbpnm] [PATCH] Fix sign compare error when checking file size
  2026-11-11  1:25 [raw2rgbpnm] [PATCH] Fix sign compare error when checking file size Paul Elder
@ 2026-11-11  8:50 ` Laurent Pinchart
  2026-11-11  9:23   ` Sakari Ailus
  0 siblings, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2026-11-11  8:50 UTC (permalink / raw)
  To: Paul Elder; +Cc: linux-media, sakari.ailus

On Wed, Nov 11, 2026 at 10:25:25AM +0900, Paul Elder wrote:
> Fix the sign compare error when checking file size by casting the file
> size returned by ftell to unsigned int, as it is a signed long being
> compared against UINT_MAX.
> 
> This fixes the following compiler error:
> 
> FAILED: [code=1] raw2rgbpnm.p/raw2rgbpnm.c.o
> /home/autobuild/autobuild/instance-5/output-1/host/bin/arm-buildroot-linux-gnueabi-gcc -Iraw2rgbpnm.p -I. -I.. -I../include -fdiagnostics-color=always -Wall -Winvalid-pch -Wextra -Werror -O3 -Wno-missing-field-initializers -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -g0 -D_FORTIFY_SOURCE=1 -MD -MQ raw2rgbpnm.p/raw2rgbpnm.c.o -MF raw2rgbpnm.p/raw2rgbpnm.c.o.d -o raw2rgbpnm.p/raw2rgbpnm.c.o -c ../raw2rgbpnm.c
> ../raw2rgbpnm.c: In function 'read_raw_data':
> ../raw2rgbpnm.c:173:36: error: comparison of integer expressions of different signedness: 'unsigned int' and 'long int' [-Werror=sign-compare]
>   173 |                 if (UINT_MAX / bpp < file_size)
>       |                                    ^
> ../raw2rgbpnm.c:185:23: error: comparison of integer expressions of different signedness: 'long int' and 'unsigned int' [-Werror=sign-compare]
>   185 |         if (file_size > UINT_MAX)
>       |                       ^
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> I've been getting build failure reports from buildroot for this.
> https://autobuild.buildroot.org/results/f1f070bc5ac72d8b0f2ed403273635b7459a9ae5/
> ---
>  raw2rgbpnm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> index 0c71be321746..39b186473624 100644
> --- a/raw2rgbpnm.c
> +++ b/raw2rgbpnm.c
> @@ -170,7 +170,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
>  	}
>  
>  	if (!*height && line_length) {
> -		if (UINT_MAX / bpp < file_size)
> +		if (UINT_MAX / bpp < (unsigned int)file_size)
>  			error("line length too large");
>  		*height = file_size / line_length;
>  		*width = line_length * 8 / bpp;
> @@ -182,7 +182,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
>  	if (!line_length || UINT_MAX / line_length < *height ||
>  	    line_length < *width * bpp / 8)
>  		error("stride %u is bad", line_length);
> -	if (file_size > UINT_MAX)
> +	if ((unsigned int)file_size > UINT_MAX)

A better option in my opinion is to turn file_size into an unsigned
variable (after checking for errors from ftell()) and drop all the
casts. There are already casts to unsigned int though, the cold is old,
and there's little incentive in refactoring it extensively, so I think
this patch is good enough.

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

>  		error("too large file");
>  	if ((unsigned int)file_size < line_length * *height)
>  		error("out of input data");

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [raw2rgbpnm] [PATCH] Fix sign compare error when checking file size
  2026-11-11  8:50 ` Laurent Pinchart
@ 2026-11-11  9:23   ` Sakari Ailus
  2026-11-11  9:27     ` Laurent Pinchart
  0 siblings, 1 reply; 5+ messages in thread
From: Sakari Ailus @ 2026-11-11  9:23 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Paul Elder, linux-media

Hi Paul, Laurent,

On Wed, Nov 11, 2026 at 10:50:14AM +0200, Laurent Pinchart wrote:
> On Wed, Nov 11, 2026 at 10:25:25AM +0900, Paul Elder wrote:
> > Fix the sign compare error when checking file size by casting the file
> > size returned by ftell to unsigned int, as it is a signed long being
> > compared against UINT_MAX.
> > 
> > This fixes the following compiler error:
> > 
> > FAILED: [code=1] raw2rgbpnm.p/raw2rgbpnm.c.o
> > /home/autobuild/autobuild/instance-5/output-1/host/bin/arm-buildroot-linux-gnueabi-gcc -Iraw2rgbpnm.p -I. -I.. -I../include -fdiagnostics-color=always -Wall -Winvalid-pch -Wextra -Werror -O3 -Wno-missing-field-initializers -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -g0 -D_FORTIFY_SOURCE=1 -MD -MQ raw2rgbpnm.p/raw2rgbpnm.c.o -MF raw2rgbpnm.p/raw2rgbpnm.c.o.d -o raw2rgbpnm.p/raw2rgbpnm.c.o -c ../raw2rgbpnm.c
> > ../raw2rgbpnm.c: In function 'read_raw_data':
> > ../raw2rgbpnm.c:173:36: error: comparison of integer expressions of different signedness: 'unsigned int' and 'long int' [-Werror=sign-compare]
> >   173 |                 if (UINT_MAX / bpp < file_size)
> >       |                                    ^
> > ../raw2rgbpnm.c:185:23: error: comparison of integer expressions of different signedness: 'long int' and 'unsigned int' [-Werror=sign-compare]
> >   185 |         if (file_size > UINT_MAX)
> >       |                       ^
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> > I've been getting build failure reports from buildroot for this.
> > https://autobuild.buildroot.org/results/f1f070bc5ac72d8b0f2ed403273635b7459a9ae5/
> > ---
> >  raw2rgbpnm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> > index 0c71be321746..39b186473624 100644
> > --- a/raw2rgbpnm.c
> > +++ b/raw2rgbpnm.c
> > @@ -170,7 +170,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
> >  	}
> >  
> >  	if (!*height && line_length) {
> > -		if (UINT_MAX / bpp < file_size)
> > +		if (UINT_MAX / bpp < (unsigned int)file_size)
> >  			error("line length too large");
> >  		*height = file_size / line_length;
> >  		*width = line_length * 8 / bpp;
> > @@ -182,7 +182,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
> >  	if (!line_length || UINT_MAX / line_length < *height ||
> >  	    line_length < *width * bpp / 8)
> >  		error("stride %u is bad", line_length);
> > -	if (file_size > UINT_MAX)
> > +	if ((unsigned int)file_size > UINT_MAX)

This will never be true, as an unsigned int won't be larger than UINT_MAX.
file_size is long so unsigned long would seem reasonable here. Although it
seems there was no bug here, it's just a compiler warning on what could
have been a bug...

> 
> A better option in my opinion is to turn file_size into an unsigned
> variable (after checking for errors from ftell()) and drop all the
> casts. There are already casts to unsigned int though, the cold is old,
> and there's little incentive in refactoring it extensively, so I think
> this patch is good enough.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> >  		error("too large file");
> >  	if ((unsigned int)file_size < line_length * *height)
> >  		error("out of input data");
> 

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [raw2rgbpnm] [PATCH] Fix sign compare error when checking file size
  2026-11-11  9:23   ` Sakari Ailus
@ 2026-11-11  9:27     ` Laurent Pinchart
  2026-11-11  9:30       ` Sakari Ailus
  0 siblings, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2026-11-11  9:27 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: Paul Elder, linux-media

On Wed, Nov 11, 2026 at 11:23:10AM +0200, Sakari Ailus wrote:
> On Wed, Nov 11, 2026 at 10:50:14AM +0200, Laurent Pinchart wrote:
> > On Wed, Nov 11, 2026 at 10:25:25AM +0900, Paul Elder wrote:
> > > Fix the sign compare error when checking file size by casting the file
> > > size returned by ftell to unsigned int, as it is a signed long being
> > > compared against UINT_MAX.
> > > 
> > > This fixes the following compiler error:
> > > 
> > > FAILED: [code=1] raw2rgbpnm.p/raw2rgbpnm.c.o
> > > /home/autobuild/autobuild/instance-5/output-1/host/bin/arm-buildroot-linux-gnueabi-gcc -Iraw2rgbpnm.p -I. -I.. -I../include -fdiagnostics-color=always -Wall -Winvalid-pch -Wextra -Werror -O3 -Wno-missing-field-initializers -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -g0 -D_FORTIFY_SOURCE=1 -MD -MQ raw2rgbpnm.p/raw2rgbpnm.c.o -MF raw2rgbpnm.p/raw2rgbpnm.c.o.d -o raw2rgbpnm.p/raw2rgbpnm.c.o -c ../raw2rgbpnm.c
> > > ../raw2rgbpnm.c: In function 'read_raw_data':
> > > ../raw2rgbpnm.c:173:36: error: comparison of integer expressions of different signedness: 'unsigned int' and 'long int' [-Werror=sign-compare]
> > >   173 |                 if (UINT_MAX / bpp < file_size)
> > >       |                                    ^
> > > ../raw2rgbpnm.c:185:23: error: comparison of integer expressions of different signedness: 'long int' and 'unsigned int' [-Werror=sign-compare]
> > >   185 |         if (file_size > UINT_MAX)
> > >       |                       ^
> > > 
> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > > ---
> > > I've been getting build failure reports from buildroot for this.
> > > https://autobuild.buildroot.org/results/f1f070bc5ac72d8b0f2ed403273635b7459a9ae5/
> > > ---
> > >  raw2rgbpnm.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> > > index 0c71be321746..39b186473624 100644
> > > --- a/raw2rgbpnm.c
> > > +++ b/raw2rgbpnm.c
> > > @@ -170,7 +170,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
> > >  	}
> > >  
> > >  	if (!*height && line_length) {
> > > -		if (UINT_MAX / bpp < file_size)
> > > +		if (UINT_MAX / bpp < (unsigned int)file_size)
> > >  			error("line length too large");
> > >  		*height = file_size / line_length;
> > >  		*width = line_length * 8 / bpp;
> > > @@ -182,7 +182,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
> > >  	if (!line_length || UINT_MAX / line_length < *height ||
> > >  	    line_length < *width * bpp / 8)
> > >  		error("stride %u is bad", line_length);
> > > -	if (file_size > UINT_MAX)
> > > +	if ((unsigned int)file_size > UINT_MAX)
> 
> This will never be true, as an unsigned int won't be larger than UINT_MAX.
> file_size is long so unsigned long would seem reasonable here. Although it
> seems there was no bug here, it's just a compiler warning on what could
> have been a bug...

What's your preference to silence this warning ? Would you like a
minimal change, or a larger cleanup ?

> > A better option in my opinion is to turn file_size into an unsigned
> > variable (after checking for errors from ftell()) and drop all the
> > casts. There are already casts to unsigned int though, the cold is old,
> > and there's little incentive in refactoring it extensively, so I think
> > this patch is good enough.
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > >  		error("too large file");
> > >  	if ((unsigned int)file_size < line_length * *height)
> > >  		error("out of input data");

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [raw2rgbpnm] [PATCH] Fix sign compare error when checking file size
  2026-11-11  9:27     ` Laurent Pinchart
@ 2026-11-11  9:30       ` Sakari Ailus
  0 siblings, 0 replies; 5+ messages in thread
From: Sakari Ailus @ 2026-11-11  9:30 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Paul Elder, linux-media

On Wed, Nov 11, 2026 at 11:27:58AM +0200, Laurent Pinchart wrote:
> On Wed, Nov 11, 2026 at 11:23:10AM +0200, Sakari Ailus wrote:
> > On Wed, Nov 11, 2026 at 10:50:14AM +0200, Laurent Pinchart wrote:
> > > On Wed, Nov 11, 2026 at 10:25:25AM +0900, Paul Elder wrote:
> > > > Fix the sign compare error when checking file size by casting the file
> > > > size returned by ftell to unsigned int, as it is a signed long being
> > > > compared against UINT_MAX.
> > > > 
> > > > This fixes the following compiler error:
> > > > 
> > > > FAILED: [code=1] raw2rgbpnm.p/raw2rgbpnm.c.o
> > > > /home/autobuild/autobuild/instance-5/output-1/host/bin/arm-buildroot-linux-gnueabi-gcc -Iraw2rgbpnm.p -I. -I.. -I../include -fdiagnostics-color=always -Wall -Winvalid-pch -Wextra -Werror -O3 -Wno-missing-field-initializers -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -g0 -D_FORTIFY_SOURCE=1 -MD -MQ raw2rgbpnm.p/raw2rgbpnm.c.o -MF raw2rgbpnm.p/raw2rgbpnm.c.o.d -o raw2rgbpnm.p/raw2rgbpnm.c.o -c ../raw2rgbpnm.c
> > > > ../raw2rgbpnm.c: In function 'read_raw_data':
> > > > ../raw2rgbpnm.c:173:36: error: comparison of integer expressions of different signedness: 'unsigned int' and 'long int' [-Werror=sign-compare]
> > > >   173 |                 if (UINT_MAX / bpp < file_size)
> > > >       |                                    ^
> > > > ../raw2rgbpnm.c:185:23: error: comparison of integer expressions of different signedness: 'long int' and 'unsigned int' [-Werror=sign-compare]
> > > >   185 |         if (file_size > UINT_MAX)
> > > >       |                       ^
> > > > 
> > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > > > ---
> > > > I've been getting build failure reports from buildroot for this.
> > > > https://autobuild.buildroot.org/results/f1f070bc5ac72d8b0f2ed403273635b7459a9ae5/
> > > > ---
> > > >  raw2rgbpnm.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> > > > index 0c71be321746..39b186473624 100644
> > > > --- a/raw2rgbpnm.c
> > > > +++ b/raw2rgbpnm.c
> > > > @@ -170,7 +170,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
> > > >  	}
> > > >  
> > > >  	if (!*height && line_length) {
> > > > -		if (UINT_MAX / bpp < file_size)
> > > > +		if (UINT_MAX / bpp < (unsigned int)file_size)
> > > >  			error("line length too large");
> > > >  		*height = file_size / line_length;
> > > >  		*width = line_length * 8 / bpp;
> > > > @@ -182,7 +182,7 @@ static unsigned char *read_raw_data(char *filename, unsigned int *width,
> > > >  	if (!line_length || UINT_MAX / line_length < *height ||
> > > >  	    line_length < *width * bpp / 8)
> > > >  		error("stride %u is bad", line_length);
> > > > -	if (file_size > UINT_MAX)
> > > > +	if ((unsigned int)file_size > UINT_MAX)
> > 
> > This will never be true, as an unsigned int won't be larger than UINT_MAX.
> > file_size is long so unsigned long would seem reasonable here. Although it
> > seems there was no bug here, it's just a compiler warning on what could
> > have been a bug...
> 
> What's your preference to silence this warning ? Would you like a
> minimal change, or a larger cleanup ?

My suggestion was to use unsigned long instead of unsigned int.

In fact I'd move the file_size check above its first use while at it.

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-11  9:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-11  1:25 [raw2rgbpnm] [PATCH] Fix sign compare error when checking file size Paul Elder
2026-11-11  8:50 ` Laurent Pinchart
2026-11-11  9:23   ` Sakari Ailus
2026-11-11  9:27     ` Laurent Pinchart
2026-11-11  9:30       ` Sakari Ailus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox