On Fri, Jun 07, 2024 at 10:24:52AM -0500, Eric Sandeen wrote:
> Now that the HAVE_FALLOCATE guard around including
> <linux/falloc.h> in linux/xfs.h has been removed via
> 15fb447f ("configure: don't check for fallocate"),
> bad things can happen because we reference fallocate in
> <xfs/linux.h> without defining _GNU_SOURCE:
>
> $ cat test.c
> #include <xfs/linux.h>
>
> int main(void)
> {
> return 0;
> }
>
> $ gcc -o test test.c
> In file included from test.c:1:
> /usr/include/xfs/linux.h: In function ‘platform_zero_range’:
> /usr/include/xfs/linux.h:186:15: error: implicit declaration of function ‘fallocate’ [-Wimplicit-function-declaration]
> 186 | ret = fallocate(fd, FALLOC_FL_ZERO_RANGE, start, len);
> | ^~~~~~~~~
>
> i.e. xfs/linux.h includes fcntl.h without _GNU_SOURCE, so we
> don't get an fallocate prototype.
>
> Rather than playing games with header files, just remove the
> platform_zero_range() wrapper - we have only one platform, and
> only one caller after all - and simply call fallocate directly
> if we have the FALLOC_FL_ZERO_RANGE flag defined.
>
> (LTP also runs into this sort of problem at configure time ...)
>
> Darrick points out that this changes a public header, but
> platform_zero_range() has only been exposed by default
> (without the oddball / internal xfsprogs guard) for a couple
> of xfsprogs releases, so it's quite unlikely that anyone is
> using this oddball fallocate wrapper.
>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Ok I'm convinced
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
>
> V2: remove error variable, add to commit msg
> V3: Drop FALLOC_FL_ZERO_RANGE #ifdef per hch's suggestion and
> add his RVB from V2, with changes.
>
> NOTE: compile tested only
>
> diff --git a/include/linux.h b/include/linux.h
> index 95a0deee..a13072d2 100644
> --- a/include/linux.h
> +++ b/include/linux.h
> @@ -174,24 +174,6 @@ static inline void platform_mntent_close(struct mntent_cursor * cursor)
> endmntent(cursor->mtabp);
> }
>
> -#if defined(FALLOC_FL_ZERO_RANGE)
> -static inline int
> -platform_zero_range(
> - int fd,
> - xfs_off_t start,
> - size_t len)
> -{
> - int ret;
> -
> - ret = fallocate(fd, FALLOC_FL_ZERO_RANGE, start, len);
> - if (!ret)
> - return 0;
> - return -errno;
> -}
> -#else
> -#define platform_zero_range(fd, s, l) (-EOPNOTSUPP)
> -#endif
> -
> /*
> * Use SIGKILL to simulate an immediate program crash, without a chance to run
> * atexit handlers.
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 153007d5..b54505b5 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -73,7 +73,7 @@ libxfs_device_zero(struct xfs_buftarg *btp, xfs_daddr_t start, uint len)
>
> /* try to use special zeroing methods, fall back to writes if needed */
> len_bytes = LIBXFS_BBTOOFF64(len);
> - error = platform_zero_range(fd, start_offset, len_bytes);
> + error = fallocate(fd, FALLOC_FL_ZERO_RANGE, start_offset, len_bytes);
> if (!error) {
> xfs_buftarg_trip_write(btp);
> return 0;
>
>
