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

[Sakari Ailus <sakari.ailus@iki.fi>]

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