Re: [PATCH] xfsprogs: Fix mismatched return type of filesize()

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Apr 01, 2025 at 08:22:33PM -0400, Theodore Ts'o wrote:
> On Fri, Feb 21, 2025 at 07:57:57PM +0100, Pavel Reichl wrote:
> > The function filesize() was declared with a return type of 'long' but
> > defined with 'off_t'. This mismatch caused build issues due to type
> > incompatibility.
> > 
> > This commit updates the declaration to match the definition, ensuring
> > consistency and preventing potential compilation errors.
> > 
> > Fixes: 73fb78e5ee8 ("mkfs: support copying in large or sparse files")
> 
> I had run into this issue when building xfsprogs on i386, and had
> investigated the compilation failure before finding this commit in
> origin/for-next.  But in my fix, I also found that there was a missing
> long -> off_t conversion in setup_proto():
> 
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index 7f56a3d8..52ef64ff 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -61,7 +61,7 @@ setup_proto(
>  	char		*buf = NULL;
>  	static char	dflt[] = "d--755 0 0 $";
>  	int		fd;
> -	long		size;
> +	off_t		size;
>  
>  	if (!fname)
>  		return dflt;
> 
> ... since setup_proto() also calls filesize():
> 
> 	if ((fd = open(fname, O_RDONLY)) < 0 || (size = filesize(fd)) < 0) {
> 
> How important is it fix this up?  I can send a formal patch if that
> would be helpful, but commit a5466cee9874 is certainly enough to fix
> the build failure so maybe it's enough.

Yes, this is important -- off_t can be larger than long, and that can
result in incorrect truncations.  I hope that nobody will ever pass mkfs
a 5GB protofile on 32-bit, but at least the C type usage could be
correct.

--D

> 
> Cheers,
> 
> 					- Ted
>