This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] <sys/stat.h>: Use Linux kernel UAPI header if available and useful
On 6/11/19 1:36 PM, Florian Weimer wrote:
> * Andreas Schwab:
>
>> On Jun 11 2019, Florian Weimer <fweimer@redhat.com> wrote:
>>
>>> I can implement the preprocessor hack as well if you want me to.
>>
>> Yes, that's what I meant with non-changing. statx_generic is using a
>> fixed subset of statx and thus should only use the fallback definition
>> of struct statx.
>
> The patch below seems to work. I'm still re-testing it with old kernel
> headers.
>
> Thanks,
> Florian
>
> <sys/stat.h>: Use Linux kernel UAPI header if available and useful
>
> This will automatically import new STATX_* constants. It also avoids
> a conflict between <sys/stat.h> and <linux/stat.h>.
>
> 2019-06-11 Florian Weimer <fweimer@redhat.com>
>
> Linux: Use kernel headers for statx definitions if available.
> * include/bits/statx-generic.h: New file.
> * include/bits/types/struct_statx.h: Likewise.
> * include/bits/types/struct_statx_timestamp.h: Likewise.
> * io/Makefile (headers): Add bits/statx-generic.h.
> * io/bits/statx-generic.h: New file. Partly copied from
> io/bits/statx.h.
> * io/statx_generic.c: Include <bits/types/struct_statx.h> to
> define original_statx.
> * io/bits/types/struct_statx.h: Likewise.
> * io/bits/types/struct_statx_timestamp.h: Likewise.
> (statx_generic): Use original_statx.
> * io/bits/statx.h: Rewrite to include <bits/statx-generic.h>.
> * sysdeps/unix/sysv/linux/bits/statx.h: New file.
I'm OK with this for master. Andreas should also comment.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> diff --git a/include/bits/statx-generic.h b/include/bits/statx-generic.h
> new file mode 100644
> index 0000000000..21674721b6
> --- /dev/null
> +++ b/include/bits/statx-generic.h
> @@ -0,0 +1 @@
> +#include <io/bits/statx-generic.h>
OK.
> diff --git a/include/bits/types/struct_statx.h b/include/bits/types/struct_statx.h
> new file mode 100644
> index 0000000000..82add6484f
> --- /dev/null
> +++ b/include/bits/types/struct_statx.h
> @@ -0,0 +1 @@
> +#include <io/bits/types/struct_statx.h>
OK.
> diff --git a/include/bits/types/struct_statx_timestamp.h b/include/bits/types/struct_statx_timestamp.h
> new file mode 100644
> index 0000000000..9fbedd5749
> --- /dev/null
> +++ b/include/bits/types/struct_statx_timestamp.h
> @@ -0,0 +1 @@
> +#include <io/bits/types/struct_statx_timestamp.h>
OK.
> diff --git a/io/Makefile b/io/Makefile
> index f2404db041..088e86da77 100644
> --- a/io/Makefile
> +++ b/io/Makefile
> @@ -25,7 +25,9 @@ include ../Makeconfig
> headers := sys/stat.h bits/stat.h sys/statfs.h bits/statfs.h sys/vfs.h \
> sys/statvfs.h bits/statvfs.h fcntl.h sys/fcntl.h bits/fcntl.h \
> poll.h sys/poll.h bits/poll.h bits/fcntl2.h bits/poll2.h \
> - bits/statx.h utime.h ftw.h fts.h sys/sendfile.h
> + bits/statx.h bits/statx-generic.h bits/types/struct_statx.h \
> + bits/types/struct_statx_timestamp.h \
> + utime.h ftw.h fts.h sys/sendfile.h
OK.
>
> routines := \
> utime \
> diff --git a/io/bits/statx-generic.h b/io/bits/statx-generic.h
> new file mode 100644
> index 0000000000..1f5abbf148
> --- /dev/null
> +++ b/io/bits/statx-generic.h
> @@ -0,0 +1,60 @@
> +/* Generic statx-related definitions and declarations.
> + Copyright (C) 2018-2019 Free Software Foundation, Inc.
s/2018-2019/2019/g
This looks like a new-from-scratch file even if it's based on the
other stat.h header.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +/* This interface is based on <linux/stat.h> in Linux. */
> +
> +#ifndef _SYS_STAT_H
> +# error Never include <bits/statx-generic.h> directly, include <sys/stat.h> instead.
> +#endif
> +
> +#include <bits/types/struct_statx_timestamp.h>
> +#include <bits/types/struct_statx.h>
OK, include types.
> +
> +#ifndef STATX_TYPE
> +# define STATX_TYPE 0x0001U
> +# define STATX_MODE 0x0002U
> +# define STATX_NLINK 0x0004U
> +# define STATX_UID 0x0008U
> +# define STATX_GID 0x0010U
> +# define STATX_ATIME 0x0020U
> +# define STATX_MTIME 0x0040U
> +# define STATX_CTIME 0x0080U
> +# define STATX_INO 0x0100U
> +# define STATX_SIZE 0x0200U
> +# define STATX_BLOCKS 0x0400U
> +# define STATX_BASIC_STATS 0x07ffU
> +# define STATX_ALL 0x0fffU
> +# define STATX_BTIME 0x0800U
> +# define STATX__RESERVED 0x80000000U
> +
> +# define STATX_ATTR_COMPRESSED 0x0004
> +# define STATX_ATTR_IMMUTABLE 0x0010
> +# define STATX_ATTR_APPEND 0x0020
> +# define STATX_ATTR_NODUMP 0x0040
> +# define STATX_ATTR_ENCRYPTED 0x0800
> +# define STATX_ATTR_AUTOMOUNT 0x1000
> +#endif /* !STATX_TYPE */
> +
> +__BEGIN_DECLS
> +
> +/* Fill *BUF with information about PATH in DIRFD. */
> +int statx (int __dirfd, const char *__restrict __path, int __flags,
> + unsigned int __mask, struct statx *__restrict __buf)
> + __THROW __nonnull ((2, 5));
> +
> +__END_DECLS
OK.
> diff --git a/io/bits/statx.h b/io/bits/statx.h
> index cff14b2543..b3147bfa8a 100644
> --- a/io/bits/statx.h
> +++ b/io/bits/statx.h
> @@ -1,4 +1,4 @@
> -/* statx-related definitions and declarations.
> +/* statx-related definitions and declarations. Generic version.
OK.
> Copyright (C) 2018-2019 Free Software Foundation, Inc.
> This file is part of the GNU C Library.
>
> @@ -19,73 +19,8 @@
> /* This interface is based on <linux/stat.h> in Linux. */
>
> #ifndef _SYS_STAT_H
> -# error Never include <bits/stat.x.h> directly, include <sys/stat.h> instead.
> +# error Never include <bits/statx.h> directly, include <sys/stat.h> instead.
OK.
> #endif
>
> -struct statx_timestamp
> -{
> - __int64_t tv_sec;
> - __uint32_t tv_nsec;
> - __int32_t __statx_timestamp_pad1[1];
> -};
> -
> -/* Warning: The kernel may add additional fields to this struct in the
> - future. Only use this struct for calling the statx function, not
> - for storing data. (Expansion will be controlled by the mask
> - argument of the statx function.) */
> -struct statx
> -{
> - __uint32_t stx_mask;
> - __uint32_t stx_blksize;
> - __uint64_t stx_attributes;
> - __uint32_t stx_nlink;
> - __uint32_t stx_uid;
> - __uint32_t stx_gid;
> - __uint16_t stx_mode;
> - __uint16_t __statx_pad1[1];
> - __uint64_t stx_ino;
> - __uint64_t stx_size;
> - __uint64_t stx_blocks;
> - __uint64_t stx_attributes_mask;
> - struct statx_timestamp stx_atime;
> - struct statx_timestamp stx_btime;
> - struct statx_timestamp stx_ctime;
> - struct statx_timestamp stx_mtime;
> - __uint32_t stx_rdev_major;
> - __uint32_t stx_rdev_minor;
> - __uint32_t stx_dev_major;
> - __uint32_t stx_dev_minor;
> - __uint64_t __statx_pad2[14];
> -};
> -
> -#define STATX_TYPE 0x0001U
> -#define STATX_MODE 0x0002U
> -#define STATX_NLINK 0x0004U
> -#define STATX_UID 0x0008U
> -#define STATX_GID 0x0010U
> -#define STATX_ATIME 0x0020U
> -#define STATX_MTIME 0x0040U
> -#define STATX_CTIME 0x0080U
> -#define STATX_INO 0x0100U
> -#define STATX_SIZE 0x0200U
> -#define STATX_BLOCKS 0x0400U
> -#define STATX_BASIC_STATS 0x07ffU
> -#define STATX_ALL 0x0fffU
> -#define STATX_BTIME 0x0800U
> -#define STATX__RESERVED 0x80000000U
> -
> -#define STATX_ATTR_COMPRESSED 0x0004
> -#define STATX_ATTR_IMMUTABLE 0x0010
> -#define STATX_ATTR_APPEND 0x0020
> -#define STATX_ATTR_NODUMP 0x0040
> -#define STATX_ATTR_ENCRYPTED 0x0800
> -#define STATX_ATTR_AUTOMOUNT 0x1000
> -
> -__BEGIN_DECLS
> -
> -/* Fill *BUF with information about PATH in DIRFD. */
> -int statx (int __dirfd, const char *__restrict __path, int __flags,
> - unsigned int __mask, struct statx *__restrict __buf)
> - __THROW __nonnull ((2, 5));
> -
> -__END_DECLS
> +/* Use the generic definitions. */
> +#include <bits/statx-generic.h>
OK.
> diff --git a/io/bits/types/struct_statx.h b/io/bits/types/struct_statx.h
> new file mode 100644
> index 0000000000..4f3ae3ece6
> --- /dev/null
> +++ b/io/bits/types/struct_statx.h
> @@ -0,0 +1,55 @@
> +/* Definition of the generic version of struct statx.
> + Copyright (C) 2018-2019 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_STAT_H
> +# error Never include <bits/types/struct_statx.h> directly, include <sys/stat.h> instead.
> +#endif
> +
> +#ifndef __statx_defined
> +#define __statx_defined 1
> +
> +/* Warning: The kernel may add additional fields to this struct in the
> + future. Only use this struct for calling the statx function, not
> + for storing data. (Expansion will be controlled by the mask
> + argument of the statx function.) */
> +struct statx
> +{
> + __uint32_t stx_mask;
> + __uint32_t stx_blksize;
> + __uint64_t stx_attributes;
> + __uint32_t stx_nlink;
> + __uint32_t stx_uid;
> + __uint32_t stx_gid;
> + __uint16_t stx_mode;
> + __uint16_t __statx_pad1[1];
> + __uint64_t stx_ino;
> + __uint64_t stx_size;
> + __uint64_t stx_blocks;
> + __uint64_t stx_attributes_mask;
> + struct statx_timestamp stx_atime;
> + struct statx_timestamp stx_btime;
> + struct statx_timestamp stx_ctime;
> + struct statx_timestamp stx_mtime;
> + __uint32_t stx_rdev_major;
> + __uint32_t stx_rdev_minor;
> + __uint32_t stx_dev_major;
> + __uint32_t stx_dev_minor;
> + __uint64_t __statx_pad2[14];
> +};
OK. Verified against linux 5.2-rc4 (d1fdb6d8f6a4109a4263176c84b899076a5f8008).
> +
> +#endif /* __statx_defined */
> diff --git a/io/bits/types/struct_statx_timestamp.h b/io/bits/types/struct_statx_timestamp.h
> new file mode 100644
> index 0000000000..0f104ef84e
> --- /dev/null
> +++ b/io/bits/types/struct_statx_timestamp.h
> @@ -0,0 +1,33 @@
> +/* Definition of the generic version of struct statx_timestamp.
> + Copyright (C) 2018-2019 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_STAT_H
> +# error Never include <bits/types/struct_statx_timestamp.h> directly, include <sys/stat.h> instead.
> +#endif
> +
> +#ifndef __statx_timestamp_defined
> +#define __statx_timestamp_defined 1
> +
> +struct statx_timestamp
> +{
> + __int64_t tv_sec;
> + __uint32_t tv_nsec;
> + __int32_t __statx_timestamp_pad1[1];
> +};
OK. Verified likewise.
> +
> +#endif /* __statx_timestamp_defined */
> diff --git a/io/statx_generic.c b/io/statx_generic.c
> index 10225ef5cb..ddc4097249 100644
> --- a/io/statx_generic.c
> +++ b/io/statx_generic.c
> @@ -18,9 +18,16 @@
>
> #include <errno.h>
> #include <fcntl.h>
> +#include <string.h>
> #include <sys/stat.h>
> #include <sys/sysmacros.h>
>
> +/* Obtain the original definition of struct statx. */
> +#undef __statx_defined
> +#define statx original_statx
> +#include <bits/types/struct_statx.h>
> +#undef statx
> +
> static inline struct statx_timestamp
> statx_convert_timestamp (struct timespec tv)
> {
> @@ -57,7 +64,7 @@ statx_generic (int fd, const char *path, int flags,
> /* The interface is defined in such a way that unused (padding)
> fields have to be cleared. STATX_BASIC_STATS corresponds to the
> data which is available via fstatat64. */
> - *buf = (struct statx)
> + struct original_statx obuf =
> {
> .stx_mask = STATX_BASIC_STATS,
> .stx_blksize = st.st_blksize,
> @@ -76,6 +83,8 @@ statx_generic (int fd, const char *path, int flags,
> .stx_dev_major = major (st.st_dev),
> .stx_dev_minor = minor (st.st_dev),
> };
> + _Static_assert (sizeof (*buf) >= sizeof (obuf), "struct statx size");
> + memcpy (buf, &obuf, sizeof (obuf));
OK.
>
> return 0;
> }
> diff --git a/sysdeps/unix/sysv/linux/bits/statx.h b/sysdeps/unix/sysv/linux/bits/statx.h
> new file mode 100644
> index 0000000000..d36f44efc6
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/statx.h
> @@ -0,0 +1,34 @@
> +/* statx-related definitions and declarations. Linux version.
> + Copyright (C) 2018-2019 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +/* This interface is based on <linux/stat.h> in Linux. */
> +
> +#ifndef _SYS_STAT_H
> +# error Never include <bits/statx.h> directly, include <sys/stat.h> instead.
> +#endif
> +
> +/* Use the Linux kernel header if available. */
> +#if __glibc_has_include (<linux/stat.h>)
So if __has_include is not defined this is all #if 0, and removed.
So in the use case that the compiler is new enough:
- If sys/stat.h is included first we check to see if linux/stat.h file is
includeable and then do so, and avoid including our own definitions.
- If linux/stat.h is included first we again detect it is includeable,
include it again (idempotent), and avoid our own definitions.
So we reduce the problem to one of "always try to include the kernel
defines first."
Works for me.
> +# include <linux/stat.h>
> +# ifdef STATX_TYPE
> +# define __statx_timestamp_defined 1
> +# define __statx_defined 1
> +# endif
> +#endif
> +
> +#include <bits/statx-generic.h>
OK.
>
--
Cheers,
Carlos.