Re: [LTP] [PATCH 1/2] lapi/fs: Include lapi/fcntl.h + define _GNU_SOURCE

[Jan Stancek <jstancek@redhat.com>]

On Tue, Apr 23, 2024 at 3:28 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> This fixes build error on musl (alpine):
>
> In file included from unlink09.c:18:
> ../../../../include/lapi/fs.h:58:15: error: unknown type name 'loff_t'
>    58 | static inline loff_t tst_max_lfs_filesize(void)
>
> loff_t is defined in <fcntl.h> (but guarded _GNU_SOURCE), but just for
> safety include lapi/fcntl.h in case lapi/fs.h is included in test which
> needs fallback definitions from lapi/fs.h.

You probably meant lapi/fcntl.h here ^^

>
> Because we require _GNU_SOURCE definition for code in lapi/fs.h, that's
> why there is the definition in both unlink09.c (the actual fix) and
> lapi/fs.h for visibility of the problem.
>
> Fixes: 2cf78f47a ("unlink: Add error tests for EPERM and EROFS")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> #define _GNU_SOURCE pain again. Would you solve it differently?

I'd likely go similar route, but I'd drop the hunk from unlink09.c.
The test is not using loff_t directly, it includes a header, so it
should be up to
that header to work without pre-existing defines.

>
>  include/lapi/fs.h                           | 5 ++++-
>  testcases/kernel/syscalls/unlink/unlink09.c | 2 ++
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/include/lapi/fs.h b/include/lapi/fs.h
> index c19ee821d..4680f0090 100644
> --- a/include/lapi/fs.h
> +++ b/include/lapi/fs.h
> @@ -9,15 +9,18 @@
>  #ifndef LAPI_FS_H__
>  #define LAPI_FS_H__
>
> +#define _GNU_SOURCE /* loff_t in <fcntl.h> */

I'd also add to comment here that it's included via lapi/fcntl.h

> +
>  #include "config.h"
> +
>  #ifndef HAVE_MOUNT_SETATTR
>  # ifdef HAVE_LINUX_FS_H
>  #  include <linux/fs.h>
>  # endif
>  #endif
>
> -#include <sys/user.h>
>  #include <limits.h>
> +#include "lapi/fcntl.h"
>  #include "lapi/abisize.h"
>
>  #ifndef FS_IOC_GETFLAGS
> diff --git a/testcases/kernel/syscalls/unlink/unlink09.c b/testcases/kernel/syscalls/unlink/unlink09.c
> index cc4b4a07e..7e3fffe5c 100644
> --- a/testcases/kernel/syscalls/unlink/unlink09.c
> +++ b/testcases/kernel/syscalls/unlink/unlink09.c
> @@ -13,6 +13,8 @@
>   * - EROFS when target file is on a read-only filesystem.
>   */
>
> +#define _GNU_SOURCE /* loff_t in <fcntl.h> */
> +
>  #include <sys/ioctl.h>
>  #include "tst_test.h"
>  #include "lapi/fs.h"
> --
> 2.43.0
>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp