[LTP] [PATCH v2 0/3] Build fix undefined struct file_handle

[LTP] [PATCH v2 0/3] Build fix undefined struct file_handle

[Petr Vorel]

Hi,

changes v1->v2:
* use autotools to detect struct file_handle

Kind regards,
Petr

diff --git configure.ac configure.ac
index 06be1c094..e44e25cc6 100644
--- configure.ac
+++ configure.ac
@@ -148,6 +148,12 @@ AC_CHECK_TYPES([struct acct_v3],,,[#include <sys/acct.h>])
 AC_CHECK_TYPES([struct af_alg_iv, struct sockaddr_alg],,,[# include <linux/if_alg.h>])
 AC_CHECK_TYPES([struct fanotify_event_info_fid, struct fanotify_event_info_header],,,[#include <sys/fanotify.h>])
 AC_CHECK_TYPES([struct file_dedupe_range],,,[#include <linux/fs.h>])
+
+AC_CHECK_TYPES([struct file_handle],,,[
+#define _GNU_SOURCE
+#include <fcntl.h>
+])
+
 AC_CHECK_TYPES([struct fs_quota_statv],,,[#include <xfs/xqm.h>])
 AC_CHECK_TYPES([struct if_nextdqblk],,,[#include <linux/quota.h>])
 AC_CHECK_TYPES([struct iovec],,,[#include <sys/uio.h>])
diff --git include/lapi/fcntl.h include/lapi/fcntl.h
index ab460beb3..e08970c4f 100644
--- include/lapi/fcntl.h
+++ include/lapi/fcntl.h
@@ -141,13 +141,13 @@
 # define MAX_HANDLE_SZ	128
 #endif
 
-#ifndef HAVE_NAME_TO_HANDLE_AT
+#ifndef HAVE_STRUCT_FILE_HANDLE
 struct file_handle {
 	unsigned int handle_bytes;
 	int handle_type;
 	/* File identifier.  */
 	unsigned char f_handle[0];
 };
-#endif /* HAVE_NAME_TO_HANDLE_AT */
+#endif /* HAVE_STRUCT_FILE_HANDLE */
 
 #endif /* __LAPI_FCNTL_H__ */

Petr Vorel (3):
  lapi: Move struct file_handle into lapi/fcntl.h
  fanotify: Fix build on undefined struct file_handle
  syscalls: Remove unused include <fcntl.h>

 configure.ac                                           |  6 ++++++
 include/lapi/fcntl.h                                   | 10 ++++++++++
 include/lapi/name_to_handle_at.h                       |  9 +--------
 testcases/kernel/syscalls/fanotify/fanotify.h          |  2 +-
 testcases/kernel/syscalls/fanotify/fanotify09.c        |  1 -
 testcases/kernel/syscalls/fanotify/fanotify13.c        |  1 -
 testcases/kernel/syscalls/fanotify/fanotify15.c        |  1 -
 testcases/kernel/syscalls/fanotify/fanotify16.c        |  1 -
 .../syscalls/name_to_handle_at/name_to_handle_at01.c   |  1 -
 .../syscalls/name_to_handle_at/name_to_handle_at02.c   |  1 -
 .../syscalls/open_by_handle_at/open_by_handle_at01.c   |  1 -
 .../syscalls/open_by_handle_at/open_by_handle_at02.c   |  1 -
 12 files changed, 18 insertions(+), 17 deletions(-)

-- 
2.29.2

[LTP] [PATCH v2 1/3] lapi: Move struct file_handle into lapi/fcntl.h

[Petr Vorel]

that way it can be used in fanotify tests
(some of use the struct, but not name_to_handle_at() syscall)
and the struct is defined in <fcntl.h> anyway.

Although detection with HAVE_NAME_TO_HANDLE_AT works (at least on glibc,
musl and uclibc-ng) add proper autotools check for the struct presence.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 configure.ac                     |  6 ++++++
 include/lapi/fcntl.h             | 10 ++++++++++
 include/lapi/name_to_handle_at.h |  9 +--------
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/configure.ac b/configure.ac
index 06be1c094..e44e25cc6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -148,6 +148,12 @@ AC_CHECK_TYPES([struct acct_v3],,,[#include <sys/acct.h>])
 AC_CHECK_TYPES([struct af_alg_iv, struct sockaddr_alg],,,[# include <linux/if_alg.h>])
 AC_CHECK_TYPES([struct fanotify_event_info_fid, struct fanotify_event_info_header],,,[#include <sys/fanotify.h>])
 AC_CHECK_TYPES([struct file_dedupe_range],,,[#include <linux/fs.h>])
+
+AC_CHECK_TYPES([struct file_handle],,,[
+#define _GNU_SOURCE
+#include <fcntl.h>
+])
+
 AC_CHECK_TYPES([struct fs_quota_statv],,,[#include <xfs/xqm.h>])
 AC_CHECK_TYPES([struct if_nextdqblk],,,[#include <linux/quota.h>])
 AC_CHECK_TYPES([struct iovec],,,[#include <sys/uio.h>])
diff --git a/include/lapi/fcntl.h b/include/lapi/fcntl.h
index d6665915f..e08970c4f 100644
--- a/include/lapi/fcntl.h
+++ b/include/lapi/fcntl.h
@@ -6,6 +6,7 @@
 #ifndef __LAPI_FCNTL_H__
 #define __LAPI_FCNTL_H__
 
+#include "config.h"
 #include <fcntl.h>
 #include <sys/socket.h>
 
@@ -140,4 +141,13 @@
 # define MAX_HANDLE_SZ	128
 #endif
 
+#ifndef HAVE_STRUCT_FILE_HANDLE
+struct file_handle {
+	unsigned int handle_bytes;
+	int handle_type;
+	/* File identifier.  */
+	unsigned char f_handle[0];
+};
+#endif /* HAVE_STRUCT_FILE_HANDLE */
+
 #endif /* __LAPI_FCNTL_H__ */
diff --git a/include/lapi/name_to_handle_at.h b/include/lapi/name_to_handle_at.h
index 3484133d1..275db4ae0 100644
--- a/include/lapi/name_to_handle_at.h
+++ b/include/lapi/name_to_handle_at.h
@@ -15,13 +15,6 @@
 #include "tst_test.h"
 
 #ifndef HAVE_NAME_TO_HANDLE_AT
-struct file_handle {
-	unsigned int handle_bytes;
-	int handle_type;
-	/* File identifier.  */
-	unsigned char f_handle[0];
-};
-
 static inline int name_to_handle_at(int dfd, const char *pathname,
                                     struct file_handle *handle,
                                     int *mount_id, int flags)
@@ -35,7 +28,7 @@ static inline int open_by_handle_at(int mount_fd, struct file_handle *handle,
 {
 	return tst_syscall(__NR_open_by_handle_at, mount_fd, handle, flags);
 }
-#endif
+#endif /* HAVE_NAME_TO_HANDLE_AT */
 
 /* Returns a valid pointer on success, NULL on errors */
 static inline struct file_handle *
-- 
2.29.2

[LTP] [PATCH v2 2/3] fanotify: Fix build on undefined struct file_handle

[Petr Vorel]

This fixes error:
fanotify09.c:201:32: error: dereferencing pointer to incomplete type ?struct file_handle?
  201 |  filename = (char *)file_handle->f_handle + file_handle->handle_bytes;

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/fanotify/fanotify.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 8907db052..039379961 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -12,8 +12,8 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <errno.h>
-#include <fcntl.h>
 #include <sys/fanotify.h>
+#include "lapi/fcntl.h"
 
 int safe_fanotify_init(const char *file, const int lineno,
 	unsigned int flags, unsigned int event_f_flags)
-- 
2.29.2

[LTP] [PATCH v2 3/3] syscalls: Remove unused include <fcntl.h>

[Petr Vorel]

Tests should always use lapi/fcntl.h instead of <fcntl.h> to fix
possible missing definitions.

But in this case removing include, because fanotify tests include
<fcntl.h> in lapi/fcntl.h (via fanotify.h) and
{name_to,open_by}_handle_at tests include lapi/fcntl.h in
lapi/name_to_handle_at.h.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/fanotify/fanotify09.c                  | 1 -
 testcases/kernel/syscalls/fanotify/fanotify13.c                  | 1 -
 testcases/kernel/syscalls/fanotify/fanotify15.c                  | 1 -
 testcases/kernel/syscalls/fanotify/fanotify16.c                  | 1 -
 .../kernel/syscalls/name_to_handle_at/name_to_handle_at01.c      | 1 -
 .../kernel/syscalls/name_to_handle_at/name_to_handle_at02.c      | 1 -
 .../kernel/syscalls/open_by_handle_at/open_by_handle_at01.c      | 1 -
 .../kernel/syscalls/open_by_handle_at/open_by_handle_at02.c      | 1 -
 8 files changed, 8 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index 30e212f44..918e40274 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -30,7 +30,6 @@
 #include <stdio.h>
 #include <sys/stat.h>
 #include <sys/types.h>
-#include <fcntl.h>
 #include <errno.h>
 #include <string.h>
 #include <sys/mount.h>
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index c9cf10555..6d812cdd1 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -21,7 +21,6 @@
 #include <sys/statfs.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <fcntl.h>
 #include <errno.h>
 #include <unistd.h>
 #include "tst_test.h"
diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
index ba8259c7c..fe143823e 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify15.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -19,7 +19,6 @@
 
 #include <string.h>
 #include <errno.h>
-#include <fcntl.h>
 #include <sys/statfs.h>
 #include <sys/types.h>
 #include "tst_test.h"
diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
index 5ffaec92f..c4b8a5abc 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify16.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -18,7 +18,6 @@
 #include <stdio.h>
 #include <sys/stat.h>
 #include <sys/types.h>
-#include <fcntl.h>
 #include <errno.h>
 #include <string.h>
 #include <sys/mount.h>
diff --git a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c
index 84ac32eab..1ac9d8214 100644
--- a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c
+++ b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c
@@ -15,7 +15,6 @@
 \*/
 
 #define _GNU_SOURCE
-#include <fcntl.h>
 #include <sys/stat.h>
 #include "lapi/name_to_handle_at.h"
 
diff --git a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c
index 7c0d57485..020b25531 100644
--- a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c
+++ b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c
@@ -10,7 +10,6 @@
 \*/
 
 #define _GNU_SOURCE
-#include <fcntl.h>
 #include "lapi/name_to_handle_at.h"
 
 #define TEST_FILE "test_file"
diff --git a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c
index c1b08f1b8..0d09e1ed8 100644
--- a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c
+++ b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c
@@ -15,7 +15,6 @@
 \*/
 
 #define _GNU_SOURCE
-#include <fcntl.h>
 #include <sys/stat.h>
 #include "lapi/name_to_handle_at.h"
 
diff --git a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c
index 3c8f06d85..0f60752c4 100644
--- a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c
+++ b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c
@@ -10,7 +10,6 @@
 \*/
 #define _GNU_SOURCE
 #include <linux/capability.h>
-#include <fcntl.h>
 #include "tst_capability.h"
 #include "lapi/name_to_handle_at.h"
 
-- 
2.29.2

[LTP] [PATCH v2 1/3] lapi: Move struct file_handle into lapi/fcntl.h

[Yang Xu]

Hi Petr
> that way it can be used in fanotify tests
> (some of use the struct, but not name_to_handle_at() syscall)
> and the struct is defined in<fcntl.h>  anyway.
> 
> Although detection with HAVE_NAME_TO_HANDLE_AT works (at least on glibc,
> musl and uclibc-ng) add proper autotools check for the struct presence.
> 
> Signed-off-by: Petr Vorel<pvorel@suse.cz>
> ---
>   configure.ac                     |  6 ++++++
>   include/lapi/fcntl.h             | 10 ++++++++++
>   include/lapi/name_to_handle_at.h |  9 +--------
>   3 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 06be1c094..e44e25cc6 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -148,6 +148,12 @@ AC_CHECK_TYPES([struct acct_v3],,,[#include<sys/acct.h>])
>   AC_CHECK_TYPES([struct af_alg_iv, struct sockaddr_alg],,,[# include<linux/if_alg.h>])
>   AC_CHECK_TYPES([struct fanotify_event_info_fid, struct fanotify_event_info_header],,,[#include<sys/fanotify.h>])
>   AC_CHECK_TYPES([struct file_dedupe_range],,,[#include<linux/fs.h>])
> +
> +AC_CHECK_TYPES([struct file_handle],,,[
> +#define _GNU_SOURCE
I think file_handle struct doesn't need _GNU_SOURCE macro or I miss
something?

Other than this, this patchset LGTM.
Acked-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> +#include<fcntl.h>
> +])
> +
>   AC_CHECK_TYPES([struct fs_quota_statv],,,[#include<xfs/xqm.h>])
>   AC_CHECK_TYPES([struct if_nextdqblk],,,[#include<linux/quota.h>])
>   AC_CHECK_TYPES([struct iovec],,,[#include<sys/uio.h>])
> diff --git a/include/lapi/fcntl.h b/include/lapi/fcntl.h
> index d6665915f..e08970c4f 100644
> --- a/include/lapi/fcntl.h
> +++ b/include/lapi/fcntl.h
> @@ -6,6 +6,7 @@
>   #ifndef __LAPI_FCNTL_H__
>   #define __LAPI_FCNTL_H__
> 
> +#include "config.h"
>   #include<fcntl.h>
>   #include<sys/socket.h>
> 
> @@ -140,4 +141,13 @@
>   # define MAX_HANDLE_SZ	128
>   #endif
> 
> +#ifndef HAVE_STRUCT_FILE_HANDLE
> +struct file_handle {
> +	unsigned int handle_bytes;
> +	int handle_type;
> +	/* File identifier.  */
> +	unsigned char f_handle[0];
> +};
> +#endif /* HAVE_STRUCT_FILE_HANDLE */
> +
>   #endif /* __LAPI_FCNTL_H__ */
> diff --git a/include/lapi/name_to_handle_at.h b/include/lapi/name_to_handle_at.h
> index 3484133d1..275db4ae0 100644
> --- a/include/lapi/name_to_handle_at.h
> +++ b/include/lapi/name_to_handle_at.h
> @@ -15,13 +15,6 @@
>   #include "tst_test.h"
> 
>   #ifndef HAVE_NAME_TO_HANDLE_AT
> -struct file_handle {
> -	unsigned int handle_bytes;
> -	int handle_type;
> -	/* File identifier.  */
> -	unsigned char f_handle[0];
> -};
> -
>   static inline int name_to_handle_at(int dfd, const char *pathname,
>                                       struct file_handle *handle,
>                                       int *mount_id, int flags)
> @@ -35,7 +28,7 @@ static inline int open_by_handle_at(int mount_fd, struct file_handle *handle,
>   {
>   	return tst_syscall(__NR_open_by_handle_at, mount_fd, handle, flags);
>   }
> -#endif
> +#endif /* HAVE_NAME_TO_HANDLE_AT */
> 
>   /* Returns a valid pointer on success, NULL on errors */
>   static inline struct file_handle *

[LTP] [PATCH v2 1/3] lapi: Move struct file_handle into lapi/fcntl.h

[Petr Vorel]

Hi Xu,

...
> > +AC_CHECK_TYPES([struct file_handle],,,[
> > +#define _GNU_SOURCE
> I think file_handle struct doesn't need _GNU_SOURCE macro or I miss
> something?

It does require :).
In glibc is guarded in sysdeps/unix/sysv/linux/bits/fcntl-linux.h with
__USE_GNU which is controlled by user with _GNU_SOURCE.

In musl (which is much easier to read) is just guarded by _GNU_SOURCE.

That is the reason why I guarded it with __USE_GNU in uclibc-ng, when I
backported there the implementation from musl.

Kind regards,
Petr

> Other than this, this patchset LGTM.
> Acked-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Thanks for your review!

Kind regards,
Petr
> > +#include<fcntl.h>
> > +])

[LTP] [PATCH v2 1/3] lapi: Move struct file_handle into lapi/fcntl.h

[Yang Xu]

Hi Petr
> Hi Xu,
>
> ...
>>> +AC_CHECK_TYPES([struct file_handle],,,[
>>> +#define _GNU_SOURCE
>> I think file_handle struct doesn't need _GNU_SOURCE macro or I miss
>> something?
>
> It does require :).
> In glibc is guarded in sysdeps/unix/sysv/linux/bits/fcntl-linux.h with
> __USE_GNU which is controlled by user with _GNU_SOURCE.
>
> In musl (which is much easier to read) is just guarded by _GNU_SOURCE.
>
> That is the reason why I guarded it with __USE_GNU in uclibc-ng, when I
> backported there the implementation from musl.
Thanks for your explanation, I see glibc code, you are right. I missed 
file_handle struct using _USE_GNU macro. Also, I see feature.h header 
uses _GNU_SOURCE controlling __USE_GNU.

Best Regards
Yang Xu
>
> Kind regards,
> Petr
>
>> Other than this, this patchset LGTM.
>> Acked-by: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
> Thanks for your review!
>
> Kind regards,
> Petr
>>> +#include<fcntl.h>
>>> +])
>
>
> .
>

[LTP] [PATCH v2 3/3] syscalls: Remove unused include <fcntl.h>

[Xiao Yang]

On 2021/1/13 15:51, Petr Vorel wrote:
> Tests should always use lapi/fcntl.h instead of <fcntl.h> to fix
> possible missing definitions.
>
> But in this case removing include, because fanotify tests include
> <fcntl.h> in lapi/fcntl.h (via fanotify.h) and
> {name_to,open_by}_handle_at tests include lapi/fcntl.h in
> lapi/name_to_handle_at.h.
Hi Petr,

This patchset looks good to me.
Reviewed-by: Xiao Yang <yangx.jy@cn.fujitsu.com>

Only one monir question:
Why do we remove <fcntl.h> header for all fanotify tests?
Of course, just four fanotify tests take use of struct file_handle.

Best Regards,
Xiao Yang
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  testcases/kernel/syscalls/fanotify/fanotify09.c                  | 1 -
>  testcases/kernel/syscalls/fanotify/fanotify13.c                  | 1 -
>  testcases/kernel/syscalls/fanotify/fanotify15.c                  | 1 -
>  testcases/kernel/syscalls/fanotify/fanotify16.c                  | 1 -
>  .../kernel/syscalls/name_to_handle_at/name_to_handle_at01.c      | 1 -
>  .../kernel/syscalls/name_to_handle_at/name_to_handle_at02.c      | 1 -
>  .../kernel/syscalls/open_by_handle_at/open_by_handle_at01.c      | 1 -
>  .../kernel/syscalls/open_by_handle_at/open_by_handle_at02.c      | 1 -
>  8 files changed, 8 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index 30e212f44..918e40274 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -30,7 +30,6 @@
>  #include <stdio.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
> -#include <fcntl.h>
>  #include <errno.h>
>  #include <string.h>
>  #include <sys/mount.h>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
> index c9cf10555..6d812cdd1 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify13.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
> @@ -21,7 +21,6 @@
>  #include <sys/statfs.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> -#include <fcntl.h>
>  #include <errno.h>
>  #include <unistd.h>
>  #include "tst_test.h"
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
> index ba8259c7c..fe143823e 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify15.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
> @@ -19,7 +19,6 @@
>  
>  #include <string.h>
>  #include <errno.h>
> -#include <fcntl.h>
>  #include <sys/statfs.h>
>  #include <sys/types.h>
>  #include "tst_test.h"
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
> index 5ffaec92f..c4b8a5abc 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify16.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
> @@ -18,7 +18,6 @@
>  #include <stdio.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
> -#include <fcntl.h>
>  #include <errno.h>
>  #include <string.h>
>  #include <sys/mount.h>
> diff --git a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c
> index 84ac32eab..1ac9d8214 100644
> --- a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c
> +++ b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c
> @@ -15,7 +15,6 @@
>  \*/
>  
>  #define _GNU_SOURCE
> -#include <fcntl.h>
>  #include <sys/stat.h>
>  #include "lapi/name_to_handle_at.h"
>  
> diff --git a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c
> index 7c0d57485..020b25531 100644
> --- a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c
> +++ b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c
> @@ -10,7 +10,6 @@
>  \*/
>  
>  #define _GNU_SOURCE
> -#include <fcntl.h>
>  #include "lapi/name_to_handle_at.h"
>  
>  #define TEST_FILE "test_file"
> diff --git a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c
> index c1b08f1b8..0d09e1ed8 100644
> --- a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c
> +++ b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c
> @@ -15,7 +15,6 @@
>  \*/
>  
>  #define _GNU_SOURCE
> -#include <fcntl.h>
>  #include <sys/stat.h>
>  #include "lapi/name_to_handle_at.h"
>  
> diff --git a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c
> index 3c8f06d85..0f60752c4 100644
> --- a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c
> +++ b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c
> @@ -10,7 +10,6 @@
>  \*/
>  #define _GNU_SOURCE
>  #include <linux/capability.h>
> -#include <fcntl.h>
>  #include "tst_capability.h"
>  #include "lapi/name_to_handle_at.h"
>  

[LTP] [PATCH v2 3/3] syscalls: Remove unused include <fcntl.h>

[Xiao Yang]

? 2021/1/13 17:16, Xiao Yang ??:
> On 2021/1/13 15:51, Petr Vorel wrote:
>> Tests should always use lapi/fcntl.h instead of<fcntl.h>  to fix
>> possible missing definitions.
>>
>> But in this case removing include, because fanotify tests include
>> <fcntl.h>  in lapi/fcntl.h (via fanotify.h) and
>> {name_to,open_by}_handle_at tests include lapi/fcntl.h in
>> lapi/name_to_handle_at.h.
> Hi Petr,
>
> This patchset looks good to me.
> Reviewed-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>
> Only one monir question:
> Why do we remove<fcntl.h>  header for all fanotify tests?

Sorry, correct one word(do =>don't)

> Of course, just four fanotify tests take use of struct file_handle.
>
> Best Regards,
> Xiao Yang
>> Signed-off-by: Petr Vorel<pvorel@suse.cz>
>> ---
>>   testcases/kernel/syscalls/fanotify/fanotify09.c                  | 1 -
>>   testcases/kernel/syscalls/fanotify/fanotify13.c                  | 1 -
>>   testcases/kernel/syscalls/fanotify/fanotify15.c                  | 1 -
>>   testcases/kernel/syscalls/fanotify/fanotify16.c                  | 1 -
>>   .../kernel/syscalls/name_to_handle_at/name_to_handle_at01.c      | 1 -
>>   .../kernel/syscalls/name_to_handle_at/name_to_handle_at02.c      | 1 -
>>   .../kernel/syscalls/open_by_handle_at/open_by_handle_at01.c      | 1 -
>>   .../kernel/syscalls/open_by_handle_at/open_by_handle_at02.c      | 1 -
>>   8 files changed, 8 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
>> index 30e212f44..918e40274 100644
>> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
>> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
>> @@ -30,7 +30,6 @@
>>   #include<stdio.h>
>>   #include<sys/stat.h>
>>   #include<sys/types.h>
>> -#include<fcntl.h>
>>   #include<errno.h>
>>   #include<string.h>
>>   #include<sys/mount.h>
>> diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
>> index c9cf10555..6d812cdd1 100644
>> --- a/testcases/kernel/syscalls/fanotify/fanotify13.c
>> +++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
>> @@ -21,7 +21,6 @@
>>   #include<sys/statfs.h>
>>   #include<sys/types.h>
>>   #include<sys/stat.h>
>> -#include<fcntl.h>
>>   #include<errno.h>
>>   #include<unistd.h>
>>   #include "tst_test.h"
>> diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
>> index ba8259c7c..fe143823e 100644
>> --- a/testcases/kernel/syscalls/fanotify/fanotify15.c
>> +++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
>> @@ -19,7 +19,6 @@
>>
>>   #include<string.h>
>>   #include<errno.h>
>> -#include<fcntl.h>
>>   #include<sys/statfs.h>
>>   #include<sys/types.h>
>>   #include "tst_test.h"
>> diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
>> index 5ffaec92f..c4b8a5abc 100644
>> --- a/testcases/kernel/syscalls/fanotify/fanotify16.c
>> +++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
>> @@ -18,7 +18,6 @@
>>   #include<stdio.h>
>>   #include<sys/stat.h>
>>   #include<sys/types.h>
>> -#include<fcntl.h>
>>   #include<errno.h>
>>   #include<string.h>
>>   #include<sys/mount.h>
>> diff --git a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c
>> index 84ac32eab..1ac9d8214 100644
>> --- a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c
>> +++ b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c
>> @@ -15,7 +15,6 @@
>>   \*/
>>
>>   #define _GNU_SOURCE
>> -#include<fcntl.h>
>>   #include<sys/stat.h>
>>   #include "lapi/name_to_handle_at.h"
>>
>> diff --git a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c
>> index 7c0d57485..020b25531 100644
>> --- a/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c
>> +++ b/testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at02.c
>> @@ -10,7 +10,6 @@
>>   \*/
>>
>>   #define _GNU_SOURCE
>> -#include<fcntl.h>
>>   #include "lapi/name_to_handle_at.h"
>>
>>   #define TEST_FILE "test_file"
>> diff --git a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c
>> index c1b08f1b8..0d09e1ed8 100644
>> --- a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c
>> +++ b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c
>> @@ -15,7 +15,6 @@
>>   \*/
>>
>>   #define _GNU_SOURCE
>> -#include<fcntl.h>
>>   #include<sys/stat.h>
>>   #include "lapi/name_to_handle_at.h"
>>
>> diff --git a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c
>> index 3c8f06d85..0f60752c4 100644
>> --- a/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c
>> +++ b/testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c
>> @@ -10,7 +10,6 @@
>>   \*/
>>   #define _GNU_SOURCE
>>   #include<linux/capability.h>
>> -#include<fcntl.h>
>>   #include "tst_capability.h"
>>   #include "lapi/name_to_handle_at.h"
>>
>
>
>

[LTP] [PATCH v2 3/3] syscalls: Remove unused include <fcntl.h>

[Petr Vorel]

> On 2021/1/13 15:51, Petr Vorel wrote:
> > Tests should always use lapi/fcntl.h instead of <fcntl.h> to fix
> > possible missing definitions.

> > But in this case removing include, because fanotify tests include
> > <fcntl.h> in lapi/fcntl.h (via fanotify.h) and
> > {name_to,open_by}_handle_at tests include lapi/fcntl.h in
> > lapi/name_to_handle_at.h.
> Hi Petr,

> This patchset looks good to me.
> Reviewed-by: Xiao Yang <yangx.jy@cn.fujitsu.com>

> Only one monir question:
> Why do we remove <fcntl.h> header for all fanotify tests?
> Of course, just four fanotify tests take use of struct file_handle.
I thought I was clear in the commit description, but obviously I wasn't.
Previous commit adds lapi/fcntl.h to fanotify.h. And lapi/fcntl.h loads
<fcntl.h>, thus it's not needed here.

There has been a discussion in the past whether include "original" headers (e.g.
<fcntl.h>) in lapi headers (e.g. lapi/fcntl.h). I suggested to always include
this header, because we often don't test in Travis these corner cases when
things get broken due some definition missing. It's just safer to always use
lapi header. Previously lapi header was loaded only "when needed", but it gets
broken on less common libc (all but glibc) or on less common archs.
And it does not make sense to load "original" header and then lapi header.

If I remember correctly we agreed on this, but older lapi headers use the old
approach. I might send a patch to cleanup this and document it so we use the
same approach.

Kind regards,
Petr

> Best Regards,
> Xiao Yang

[LTP] [PATCH v2 0/3] Build fix undefined struct file_handle

[Cyril Hrubis]

Hi!
Looks good to me.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 3/3] syscalls: Remove unused include <fcntl.h>

[Petr Vorel]

Hi Xu,

> ? 2021/1/13 17:16, Xiao Yang ??:
> > On 2021/1/13 15:51, Petr Vorel wrote:
> > > Tests should always use lapi/fcntl.h instead of<fcntl.h>  to fix
> > > possible missing definitions.

> > > But in this case removing include, because fanotify tests include
> > > <fcntl.h>  in lapi/fcntl.h (via fanotify.h) and
> > > {name_to,open_by}_handle_at tests include lapi/fcntl.h in
> > > lapi/name_to_handle_at.h.
> > Hi Petr,

> > This patchset looks good to me.
> > Reviewed-by: Xiao Yang<yangx.jy@cn.fujitsu.com>

> > Only one monir question:
> > Why do we remove<fcntl.h>  header for all fanotify tests?

> Sorry, correct one word(do =>don't)
Right :). That actually makes sense, thanks!
I'll merge it with this change.

Kind regards,
Petr

[LTP] [PATCH v2 0/3] Build fix undefined struct file_handle

[Petr Vorel]

Hi Cyril, Xu,

> Hi!
> Looks good to me.

> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

Thanks for your reviews, merged (with change pointed out by Xu: remove <fcntl.h>
from all fanotify tests).

Kind regards,
Petr