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

Petr Vorel pvorel@suse.cz
Wed Apr 24 16:17:50 CEST 2024


Add required _GNU_SOURCE definition in lapi/fs.h to get loff_t
definition. 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/fcntl.h.

Also, instead adding _GNU_SOURCE to unlink09.c just include lapi/fs.h
after tst_test.h. But reverting the order requires also add <unistd.h>
to lapi/fcntl.h to fix missing getpagesize() on 32 bit build:

    In file included from unlink09.c:17:
    ../../../../include/lapi/fs.h: In function 'tst_max_lfs_filesize':
    ../../../../include/lapi/fs.h:66:26: error: implicit declaration of function 'getpagesize' [-Werror=implicit-function-declaration]
       66 |         long page_size = getpagesize();
	  |                          ^~~~~~~~~~~

(For some reason <unistd.h> is not required on 64 bit build.)
getpagesize() is also guarded _GNU_SOURCE.

Obviously having functions in lapi headers (or any headers) which
depends on _GNU_SOURCE brings problems. But moving
tst_max_lfs_filesize() out of lapi to lib/tst_*.c would not help either,
because here is loff_t also in the function signature which we need to
API header, which would again need _GNU_SOURCE definition.

Fixes: 2cf78f47a ("unlink: Add error tests for EPERM and EROFS")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
changes v1->v2:
* include/lapi/fs.h: Add #include <unistd.h>, more comments.
* unlink09.c: replace _GNU_SOURCE with switched lapi/fs.h and tst_test.h
  includes.

I wonder why getpagesize() requires _GNU_SOURCE only on 32 bit build
(triggered by -Werror=implicit-function-declaration).

NOTE: I was going to send patch which moves tst_max_lfs_filesize() to
lib/tst_file.c (renamed lib/tst_fill_file.c). IMHO it'd be better to
avoid static inline functions in headers (unless off_t), but there would
be _GNU_SOURCE required and I did not want to add it into include/fs.h.
Doing a cleanup in library includes and C files would be good, but let's
for now just fix the build.

Kind regards,
Petr

 include/lapi/fs.h                           | 10 +++++++++-
 testcases/kernel/syscalls/unlink/unlink09.c |  2 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/lapi/fs.h b/include/lapi/fs.h
index c19ee821d..ac558350c 100644
--- a/include/lapi/fs.h
+++ b/include/lapi/fs.h
@@ -9,15 +9,23 @@
 #ifndef LAPI_FS_H__
 #define LAPI_FS_H__
 
+/*
+ * loff_t in <fcntl.h> loaded via lapi/fcntl.h,
+ * getpagesize() in <unistd.h>.
+ */
+#define _GNU_SOURCE
+
 #include "config.h"
+#include <unistd.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..4f9d1eaaf 100644
--- a/testcases/kernel/syscalls/unlink/unlink09.c
+++ b/testcases/kernel/syscalls/unlink/unlink09.c
@@ -14,8 +14,8 @@
  */
 
 #include <sys/ioctl.h>
-#include "tst_test.h"
 #include "lapi/fs.h"
+#include "tst_test.h"
 
 #define TEST_EPERM_IMMUTABLE "test_eperm_immutable"
 #define TEST_EPERM_APPEND_ONLY "test_eperm_append_only"
-- 
2.43.0



More information about the ltp mailing list