[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