Re: [raw2rgbpnm] [PATCH] Fix sign compare error when checking file size

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

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