[PATCH v2 0/5] Fix sys/mount.h and kernel header tests

[PATCH v2 0/5] Fix sys/mount.h and kernel header tests

[Adhemerval Zanella]

The patchset removes the testing against the installed Linux version,
instead check if the required header is present.

It also fixes the sys/mount.h to be able use along kernel linux/mount.h
and linux/fs.h headers.

Adhemerval Zanella (5):
  glibcextract.py: Add compile_c_snippet
  linux: Use compile_c_snippet to check linux/pidfd.h availability
  linux: Mimic kernel defition for BLOCK_SIZE
  linux: Use compile_c_snippet to check linux/mount.h availability
  linux: Fix sys/mount.h usage with kernel headers

 scripts/glibcextract.py                      | 20 ++++++
 sysdeps/unix/sysv/linux/Makefile             |  8 +++
 sysdeps/unix/sysv/linux/sys/mount.h          | 73 +++++++++++++++++---
 sysdeps/unix/sysv/linux/tst-mount-compile.py | 66 ++++++++++++++++++
 sysdeps/unix/sysv/linux/tst-mount-consts.py  |  5 ++
 sysdeps/unix/sysv/linux/tst-pidfd-consts.py  |  8 ++-
 6 files changed, 168 insertions(+), 12 deletions(-)
 create mode 100755 sysdeps/unix/sysv/linux/tst-mount-compile.py

-- 
2.34.1

[PATCH v2 1/5] glibcextract.py: Add compile_c_snippet

[Adhemerval Zanella]

It might be used on tests to check if a snippet build with the provided
compiler and flags.
---
 scripts/glibcextract.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/scripts/glibcextract.py b/scripts/glibcextract.py
index 43ab58ffe2..71d9ea1acc 100644
--- a/scripts/glibcextract.py
+++ b/scripts/glibcextract.py
@@ -21,6 +21,7 @@ import os.path
 import re
 import subprocess
 import tempfile
+import collections
 
 
 def compute_c_consts(sym_data, cc):
@@ -173,3 +174,22 @@ def compare_macro_consts(source_1, source_2, cc, macro_re, exclude_re=None,
             if not allow_extra_2:
                 ret = 1
     return ret
+
+CompileResult = collections.namedtuple("CompileResult", "returncode output")
+
+def compile_c_snippet(snippet, cc, extra_cc_args=""):
+    """Compile and return whether the SNIPPET can be build with CC along
+       EXTRA_CC_ARGS compiler flags.  Return a CompileResult with RETURNCODE
+       being 0 for success, or the failure value and the compiler output.
+    """
+    with tempfile.TemporaryDirectory() as temp_dir:
+        c_file_name = os.path.join(temp_dir, 'test.c')
+        obj_file_name = os.path.join(temp_dir, 'test.o')
+        with open(c_file_name, 'w') as c_file:
+            c_file.write(snippet)
+        # Compilation has to be from stdin to avoid the temporary file
+        # name being written into the generated dependencies.
+        cmd = ('%s %s -c -o %s -x c - < %s' % (cc, extra_cc_args,
+            obj_file_name, c_file_name))
+        r = subprocess.run(cmd, shell=True, capture_output=True)
+        return CompileResult(r.returncode, r.stderr)
-- 
2.34.1

[PATCH v2 2/5] linux: Use compile_c_snippet to check linux/pidfd.h availability

[Adhemerval Zanella]

Instead of tying to a specific kernel version.

Checked on x86_64-linux-gnu.
---
 sysdeps/unix/sysv/linux/tst-pidfd-consts.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/tst-pidfd-consts.py b/sysdeps/unix/sysv/linux/tst-pidfd-consts.py
index e207b55eef..d732173abd 100644
--- a/sysdeps/unix/sysv/linux/tst-pidfd-consts.py
+++ b/sysdeps/unix/sysv/linux/tst-pidfd-consts.py
@@ -33,10 +33,12 @@ def main():
                         help='C compiler (including options) to use')
     args = parser.parse_args()
 
-    linux_version_headers = glibcsyscalls.linux_kernel_version(args.cc)
-    # Linux started to provide pidfd.h with 5.10.
-    if linux_version_headers < (5, 10):
+    if glibcextract.compile_c_snippet(
+            '#include <linux/pidfd.h>',
+            args.cc).returncode != 0:
         sys.exit (77)
+
+    linux_version_headers = glibcsyscalls.linux_kernel_version(args.cc)
     linux_version_glibc = (5, 19)
     sys.exit(glibcextract.compare_macro_consts(
                 '#include <sys/pidfd.h>\n',
-- 
2.34.1

[PATCH v2 3/5] linux: Mimic kernel defition for BLOCK_SIZE

[Adhemerval Zanella]

To avoid possible warnings if the kernel header is included before
sys/mount.h.
---
 sysdeps/unix/sysv/linux/sys/mount.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/unix/sysv/linux/sys/mount.h b/sysdeps/unix/sysv/linux/sys/mount.h
index f965986ba8..df6b0dbb42 100644
--- a/sysdeps/unix/sysv/linux/sys/mount.h
+++ b/sysdeps/unix/sysv/linux/sys/mount.h
@@ -27,8 +27,8 @@
 #include <stddef.h>
 #include <sys/ioctl.h>
 
-#define BLOCK_SIZE	1024
 #define BLOCK_SIZE_BITS	10
+#define BLOCK_SIZE	(1<<BLOCK_SIZE_BITS)
 
 
 /* These are the fs-independent mount-flags: up to 16 flags are
-- 
2.34.1

[PATCH v2 4/5] linux: Use compile_c_snippet to check linux/mount.h availability

[Adhemerval Zanella]

Checked on x86_64-linux-gnu.
---
 sysdeps/unix/sysv/linux/tst-mount-consts.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sysdeps/unix/sysv/linux/tst-mount-consts.py b/sysdeps/unix/sysv/linux/tst-mount-consts.py
index 4fb356310b..c6307cb5c2 100755
--- a/sysdeps/unix/sysv/linux/tst-mount-consts.py
+++ b/sysdeps/unix/sysv/linux/tst-mount-consts.py
@@ -33,6 +33,11 @@ def main():
                         help='C compiler (including options) to use')
     args = parser.parse_args()
 
+    if glibcextract.compile_c_snippet(
+            '#include <linux/mount.h>',
+            args.cc).returncode != 0:
+        sys.exit (77)
+
     linux_version_headers = glibcsyscalls.linux_kernel_version(args.cc)
     # Constants in glibc were updated to match Linux v5.19.  When glibc
     # constants are updated this value should be updated to match the
-- 
2.34.1

[PATCH v2 5/5] linux: Fix sys/mount.h usage with kernel headers

[Adhemerval Zanella]

Now that kernel exports linux/mount.h and includes it on linux/fs.h,
its definitions might clash with glibc exports sys/mount.h.  To avoid
the need to rearrange the Linux header to be always after glibc one,
the glibc sys/mount.h is changed to:

  1. Undefine the macros also used as enum constants.  This covers prior
     inclusion of <linux/mount.h> (for instance MS_RDONLY).

  2. Include <linux/mount.h> based on the usual __has_include check
     (needs to use __has_include ("linux/mount.h") to paper over GCC
     bugs.

  3. Define enum fsconfig_command only if FSOPEN_CLOEXEC is not defined.
     (FSOPEN_CLOEXEC should be a very close proxy.)

  4. Define struct mount_attr if MOUNT_ATTR_SIZE_VER0 is not defined.
     (Added in the same commit on the Linux side.)

This patch also adds some tests to check if including linux/fs.h and
linux/mount.h after and before sys/mount.h does work.

Checked on x86_64-linux-gnu.
---
 sysdeps/unix/sysv/linux/Makefile             |  8 +++
 sysdeps/unix/sysv/linux/sys/mount.h          | 71 +++++++++++++++++---
 sysdeps/unix/sysv/linux/tst-mount-compile.py | 66 ++++++++++++++++++
 3 files changed, 137 insertions(+), 8 deletions(-)
 create mode 100755 sysdeps/unix/sysv/linux/tst-mount-compile.py

diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 3f31c19e4a..60e4cb1521 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -265,6 +265,14 @@ $(objpfx)tst-mount-consts.out: ../sysdeps/unix/sysv/linux/tst-mount-consts.py
 	  < /dev/null > $@ 2>&1; $(evaluate-test)
 $(objpfx)tst-mount-consts.out: $(sysdeps-linux-python-deps)
 
+tests-special += $(objpfx)tst-mount-compile.out
+$(objpfx)tst-mount-compile.out: ../sysdeps/unix/sysv/linux/tst-mount-compile.py
+	$(sysdeps-linux-python) \
+	  ../sysdeps/unix/sysv/linux/tst-mount-compile.py \
+	    $(sysdeps-linux-python-cc) \
+	  < /dev/null > $@ 2>&1; $(evaluate-test)
+$(objpfx)tst-mount-compile.out: $(sysdeps-linux-python-deps)
+
 tst-rseq-disable-ENV = GLIBC_TUNABLES=glibc.pthread.rseq=0
 
 endif # $(subdir) == misc
diff --git a/sysdeps/unix/sysv/linux/sys/mount.h b/sysdeps/unix/sysv/linux/sys/mount.h
index df6b0dbb42..2e3fd6a7fe 100644
--- a/sysdeps/unix/sysv/linux/sys/mount.h
+++ b/sysdeps/unix/sysv/linux/sys/mount.h
@@ -27,6 +27,13 @@
 #include <stddef.h>
 #include <sys/ioctl.h>
 
+#ifdef __has_include
+# if __has_include ("linux/mount.h")
+#  include "linux/mount.h"
+# endif
+#endif
+
+
 #define BLOCK_SIZE_BITS	10
 #define BLOCK_SIZE	(1<<BLOCK_SIZE_BITS)
 
@@ -35,69 +42,98 @@
    supported  */
 enum
 {
+#undef MS_RDONLY
   MS_RDONLY = 1,		/* Mount read-only.  */
 #define MS_RDONLY	MS_RDONLY
+#undef MS_NOSUID
   MS_NOSUID = 2,		/* Ignore suid and sgid bits.  */
 #define MS_NOSUID	MS_NOSUID
+#undef MS_NODEV
   MS_NODEV = 4,			/* Disallow access to device special files.  */
 #define MS_NODEV	MS_NODEV
+#undef MS_NOEXEC
   MS_NOEXEC = 8,		/* Disallow program execution.  */
 #define MS_NOEXEC	MS_NOEXEC
+#undef MS_SYNCHRONOUS
   MS_SYNCHRONOUS = 16,		/* Writes are synced at once.  */
 #define MS_SYNCHRONOUS	MS_SYNCHRONOUS
+#undef MS_REMOUNT
   MS_REMOUNT = 32,		/* Alter flags of a mounted FS.  */
 #define MS_REMOUNT	MS_REMOUNT
+#undef MS_MANDLOCK
   MS_MANDLOCK = 64,		/* Allow mandatory locks on an FS.  */
 #define MS_MANDLOCK	MS_MANDLOCK
+#undef MS_DIRSYNC
   MS_DIRSYNC = 128,		/* Directory modifications are synchronous.  */
 #define MS_DIRSYNC	MS_DIRSYNC
+#undef MS_NOSYMFOLLOW
   MS_NOSYMFOLLOW = 256,		/* Do not follow symlinks.  */
 #define MS_NOSYMFOLLOW	MS_NOSYMFOLLOW
+#undef MS_NOATIME
   MS_NOATIME = 1024,		/* Do not update access times.  */
 #define MS_NOATIME	MS_NOATIME
+#undef MS_NODIRATIME
   MS_NODIRATIME = 2048,		/* Do not update directory access times.  */
 #define MS_NODIRATIME	MS_NODIRATIME
+#undef MS_BIND
   MS_BIND = 4096,		/* Bind directory at different place.  */
 #define MS_BIND		MS_BIND
+#undef MS_MOVE
   MS_MOVE = 8192,
 #define MS_MOVE		MS_MOVE
+#undef MS_REC
   MS_REC = 16384,
 #define MS_REC		MS_REC
+#undef MS_SILENT
   MS_SILENT = 32768,
 #define MS_SILENT	MS_SILENT
+#undef MS_POSIXACL
   MS_POSIXACL = 1 << 16,	/* VFS does not apply the umask.  */
 #define MS_POSIXACL	MS_POSIXACL
+#undef MS_UNBINDABLE
   MS_UNBINDABLE = 1 << 17,	/* Change to unbindable.  */
 #define MS_UNBINDABLE	MS_UNBINDABLE
+#undef MS_PRIVATE
   MS_PRIVATE = 1 << 18,		/* Change to private.  */
 #define MS_PRIVATE	MS_PRIVATE
+#undef MS_SLAVE
   MS_SLAVE = 1 << 19,		/* Change to slave.  */
 #define MS_SLAVE	MS_SLAVE
+#undef MS_SHARED
   MS_SHARED = 1 << 20,		/* Change to shared.  */
 #define MS_SHARED	MS_SHARED
+#undef MS_RELATIME
   MS_RELATIME = 1 << 21,	/* Update atime relative to mtime/ctime.  */
 #define MS_RELATIME	MS_RELATIME
+#undef MS_KERNMOUNT
   MS_KERNMOUNT = 1 << 22,	/* This is a kern_mount call.  */
 #define MS_KERNMOUNT	MS_KERNMOUNT
+#undef MS_I_VERSION
   MS_I_VERSION =  1 << 23,	/* Update inode I_version field.  */
 #define MS_I_VERSION	MS_I_VERSION
+#undef MS_STRICTATIME
   MS_STRICTATIME = 1 << 24,	/* Always perform atime updates.  */
 #define MS_STRICTATIME	MS_STRICTATIME
+#undef MS_LAZYTIME
   MS_LAZYTIME = 1 << 25,	/* Update the on-disk [acm]times lazily.  */
 #define MS_LAZYTIME	MS_LAZYTIME
+#undef MS_ACTIVE
   MS_ACTIVE = 1 << 30,
 #define MS_ACTIVE	MS_ACTIVE
+#undef MS_NOUSER
   MS_NOUSER = 1 << 31
 #define MS_NOUSER	MS_NOUSER
 };
 
 /* Flags that can be altered by MS_REMOUNT  */
+#undef MS_RMT_MASK
 #define MS_RMT_MASK (MS_RDONLY|MS_SYNCHRONOUS|MS_MANDLOCK|MS_I_VERSION \
 		     |MS_LAZYTIME)
 
 
 /* Magic mount flag number. Has to be or-ed to the flag values.  */
 
+#undef MS_MGC_VAL
 #define MS_MGC_VAL 0xc0ed0000	/* Magic flag number to indicate "new" flags */
 #define MS_MGC_MSK 0xffff0000	/* Magic flag number mask */
 
@@ -106,20 +142,35 @@ enum
    is probably as bad and I don't want to create yet another include
    file.  */
 
+#undef BLKROSET
 #define BLKROSET   _IO(0x12, 93) /* Set device read-only (0 = read-write).  */
+#undef BLKROGET
 #define BLKROGET   _IO(0x12, 94) /* Get read-only status (0 = read_write).  */
+#undef BLKRRPART
 #define BLKRRPART  _IO(0x12, 95) /* Re-read partition table.  */
+#undef BLKGETSIZE
 #define BLKGETSIZE _IO(0x12, 96) /* Return device size.  */
+#undef BLKFLSBUF
 #define BLKFLSBUF  _IO(0x12, 97) /* Flush buffer cache.  */
+#undef BLKRASET
 #define BLKRASET   _IO(0x12, 98) /* Set read ahead for block device.  */
+#undef BLKRAGET
 #define BLKRAGET   _IO(0x12, 99) /* Get current read ahead setting.  */
+#undef BLKFRASET
 #define BLKFRASET  _IO(0x12,100) /* Set filesystem read-ahead.  */
+#undef BLKFRAGET
 #define BLKFRAGET  _IO(0x12,101) /* Get filesystem read-ahead.  */
+#undef BLKSECTSET
 #define BLKSECTSET _IO(0x12,102) /* Set max sectors per request.  */
+#undef BLKSECTGET
 #define BLKSECTGET _IO(0x12,103) /* Get max sectors per request.  */
+#undef BLKSSZGET
 #define BLKSSZGET  _IO(0x12,104) /* Get block device sector size.  */
+#undef BLKBSZGET
 #define BLKBSZGET  _IOR(0x12,112,size_t)
+#undef BLKBSZSET
 #define BLKBSZSET  _IOW(0x12,113,size_t)
+#undef BLKGETSIZE64
 #define BLKGETSIZE64 _IOR(0x12,114,size_t) /* return device size.  */
 
 
@@ -157,6 +208,7 @@ enum
 #define MOUNT_ATTR_NOSYMFOLLOW  0x00200000 /* Do not follow symlinks.  */
 
 
+#ifndef MOUNT_ATTR_SIZE_VER0
 /* For mount_setattr.  */
 struct mount_attr
 {
@@ -165,6 +217,7 @@ struct mount_attr
   uint64_t propagation;
   uint64_t userns_fd;
 };
+#endif
 
 #define MOUNT_ATTR_SIZE_VER0    32 /* sizeof first published struct */
 
@@ -185,26 +238,28 @@ struct mount_attr
 #define FSPICK_EMPTY_PATH       0x00000008
 
 
+#ifndef FSOPEN_CLOEXEC
 /* The type of fsconfig call made.   */
 enum fsconfig_command
 {
   FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
-#define FSCONFIG_SET_FLAG FSCONFIG_SET_FLAG
+# define FSCONFIG_SET_FLAG FSCONFIG_SET_FLAG
   FSCONFIG_SET_STRING     = 1,    /* Set parameter, supplying a string value */
-#define FSCONFIG_SET_STRING FSCONFIG_SET_STRING
+# define FSCONFIG_SET_STRING FSCONFIG_SET_STRING
   FSCONFIG_SET_BINARY     = 2,    /* Set parameter, supplying a binary blob value */
-#define FSCONFIG_SET_BINARY FSCONFIG_SET_BINARY
+# define FSCONFIG_SET_BINARY FSCONFIG_SET_BINARY
   FSCONFIG_SET_PATH       = 3,    /* Set parameter, supplying an object by path */
-#define FSCONFIG_SET_PATH FSCONFIG_SET_PATH
+# define FSCONFIG_SET_PATH FSCONFIG_SET_PATH
   FSCONFIG_SET_PATH_EMPTY = 4,    /* Set parameter, supplying an object by (empty) path */
-#define FSCONFIG_SET_PATH_EMPTY FSCONFIG_SET_PATH_EMPTY
+# define FSCONFIG_SET_PATH_EMPTY FSCONFIG_SET_PATH_EMPTY
   FSCONFIG_SET_FD         = 5,    /* Set parameter, supplying an object by fd */
-#define FSCONFIG_SET_FD FSCONFIG_SET_FD
+# define FSCONFIG_SET_FD FSCONFIG_SET_FD
   FSCONFIG_CMD_CREATE     = 6,    /* Invoke superblock creation */
-#define FSCONFIG_CMD_CREATE FSCONFIG_CMD_CREATE
+# define FSCONFIG_CMD_CREATE FSCONFIG_CMD_CREATE
   FSCONFIG_CMD_RECONFIGURE = 7,   /* Invoke superblock reconfiguration */
-#define FSCONFIG_CMD_RECONFIGURE FSCONFIG_CMD_RECONFIGURE
+# define FSCONFIG_CMD_RECONFIGURE FSCONFIG_CMD_RECONFIGURE
 };
+#endif
 
 /* open_tree flags.  */
 #define OPEN_TREE_CLONE    1         /* Clone the target tree and attach the clone */
diff --git a/sysdeps/unix/sysv/linux/tst-mount-compile.py b/sysdeps/unix/sysv/linux/tst-mount-compile.py
new file mode 100755
index 0000000000..0ec74d4e0b
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-mount-compile.py
@@ -0,0 +1,66 @@
+#!/usr/bin/python3
+# Check if glibc provided sys/mount.h can be used along related kernel
+# headers.
+# Copyright (C) 2022 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
+# <https://www.gnu.org/licenses/>.
+
+import argparse
+import sys
+
+import glibcextract
+
+
+def main():
+    """The main entry point."""
+    parser = argparse.ArgumentParser(
+        description='Check if glibc provided sys/mount.h can be '
+                    ' used along related kernel headers.')
+    parser.add_argument('--cc', metavar='CC',
+                        help='C compiler (including options) to use')
+    args = parser.parse_args()
+
+    if glibcextract.compile_c_snippet(
+            '#include <linux/mount.h>',
+            args.cc).returncode != 0:
+        sys.exit (77)
+
+    def check(testname, snippet):
+        # Add -Werror to catch macro redefinitions and _ISOMAC to avoid
+        # internal glibc definitions.
+        r = glibcextract.compile_c_snippet(snippet, args.cc,
+                '-Werror -D_ISOMAC')
+        if r.returncode != 0:
+            print('error: test {}:\n{}'.format(testname, r.output.decode()))
+        return r.returncode
+
+    status = max(
+        check("sys/mount.h + linux/mount.h",
+              "#include <sys/mount.h>\n"
+              "#include <linux/mount.h>"),
+        check("sys/mount.h + linux/fs.h",
+              "#include <sys/mount.h>\n"
+              "#include <linux/fs.h>"),
+        check("linux/mount.h + sys/mount.h",
+              "#include <linux/mount.h>\n"
+              "#include <sys/mount.h>"),
+        check("linux/fs.h + sys/mount.h",
+              "#include <linux/fs.h>\n"
+              "#include <sys/mount.h>"))
+    sys.exit(status)
+
+if __name__ == '__main__':
+    main()
-- 
2.34.1

Re: [PATCH v2 1/5] glibcextract.py: Add compile_c_snippet

[Florian Weimer]

* Adhemerval Zanella:

> It might be used on tests to check if a snippet build with the provided
> compiler and flags.
> ---
>  scripts/glibcextract.py | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/scripts/glibcextract.py b/scripts/glibcextract.py
> index 43ab58ffe2..71d9ea1acc 100644
> --- a/scripts/glibcextract.py
> +++ b/scripts/glibcextract.py
> @@ -21,6 +21,7 @@ import os.path
>  import re
>  import subprocess
>  import tempfile
> +import collections

Please keep the imports sorted.

>  def compute_c_consts(sym_data, cc):
> @@ -173,3 +174,22 @@ def compare_macro_consts(source_1, source_2, cc, macro_re, exclude_re=None,
>              if not allow_extra_2:
>                  ret = 1
>      return ret
> +
> +CompileResult = collections.namedtuple("CompileResult", "returncode output")
> +
> +def compile_c_snippet(snippet, cc, extra_cc_args=""):
> +    """Compile and return whether the SNIPPET can be build with CC along
> +       EXTRA_CC_ARGS compiler flags.  Return a CompileResult with RETURNCODE
> +       being 0 for success, or the failure value and the compiler output.
> +    """
> +    with tempfile.TemporaryDirectory() as temp_dir:
> +        c_file_name = os.path.join(temp_dir, 'test.c')
> +        obj_file_name = os.path.join(temp_dir, 'test.o')
> +        with open(c_file_name, 'w') as c_file:
> +            c_file.write(snippet)
> +        # Compilation has to be from stdin to avoid the temporary file
> +        # name being written into the generated dependencies.
> +        cmd = ('%s %s -c -o %s -x c - < %s' % (cc, extra_cc_args,
> +            obj_file_name, c_file_name))
> +        r = subprocess.run(cmd, shell=True, capture_output=True)
> +        return CompileResult(r.returncode, r.stderr)

I think you can avoid shell=True and the direction if you just compile
c_file_name directly.  Sorry, missed that earlier.

Thanks,
Florian

Re: [PATCH v2 4/5] linux: Use compile_c_snippet to check linux/mount.h availability

[Florian Weimer]

* Adhemerval Zanella:

> Checked on x86_64-linux-gnu.
> ---
>  sysdeps/unix/sysv/linux/tst-mount-consts.py | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/sysdeps/unix/sysv/linux/tst-mount-consts.py b/sysdeps/unix/sysv/linux/tst-mount-consts.py
> index 4fb356310b..c6307cb5c2 100755
> --- a/sysdeps/unix/sysv/linux/tst-mount-consts.py
> +++ b/sysdeps/unix/sysv/linux/tst-mount-consts.py
> @@ -33,6 +33,11 @@ def main():
>                          help='C compiler (including options) to use')
>      args = parser.parse_args()
>  
> +    if glibcextract.compile_c_snippet(
> +            '#include <linux/mount.h>',
> +            args.cc).returncode != 0:
> +        sys.exit (77)
> +
>      linux_version_headers = glibcsyscalls.linux_kernel_version(args.cc)
>      # Constants in glibc were updated to match Linux v5.19.  When glibc
>      # constants are updated this value should be updated to match the

Okay after compile_c_snippet is in.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian

Re: [PATCH v2 3/5] linux: Mimic kernel defition for BLOCK_SIZE

[Florian Weimer]

* Adhemerval Zanella:

> To avoid possible warnings if the kernel header is included before
> sys/mount.h.
> ---
>  sysdeps/unix/sysv/linux/sys/mount.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sysdeps/unix/sysv/linux/sys/mount.h b/sysdeps/unix/sysv/linux/sys/mount.h
> index f965986ba8..df6b0dbb42 100644
> --- a/sysdeps/unix/sysv/linux/sys/mount.h
> +++ b/sysdeps/unix/sysv/linux/sys/mount.h
> @@ -27,8 +27,8 @@
>  #include <stddef.h>
>  #include <sys/ioctl.h>
>  
> -#define BLOCK_SIZE	1024
>  #define BLOCK_SIZE_BITS	10
> +#define BLOCK_SIZE	(1<<BLOCK_SIZE_BITS)

Looks good.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian

Re: [PATCH v2 1/5] glibcextract.py: Add compile_c_snippet

[Florian Weimer]

* Adhemerval Zanella:

> It might be used on tests to check if a snippet build with the provided
> compiler and flags.
> ---
>  scripts/glibcextract.py | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/scripts/glibcextract.py b/scripts/glibcextract.py
> index 43ab58ffe2..71d9ea1acc 100644
> --- a/scripts/glibcextract.py
> +++ b/scripts/glibcextract.py
> @@ -21,6 +21,7 @@ import os.path
>  import re
>  import subprocess
>  import tempfile
> +import collections
>  
>  
>  def compute_c_consts(sym_data, cc):
> @@ -173,3 +174,22 @@ def compare_macro_consts(source_1, source_2, cc, macro_re, exclude_re=None,
>              if not allow_extra_2:
>                  ret = 1
>      return ret
> +
> +CompileResult = collections.namedtuple("CompileResult", "returncode output")
> +
> +def compile_c_snippet(snippet, cc, extra_cc_args=""):
> +    """Compile and return whether the SNIPPET can be build with CC along
> +       EXTRA_CC_ARGS compiler flags.  Return a CompileResult with RETURNCODE
> +       being 0 for success, or the failure value and the compiler output.
> +    """
> +    with tempfile.TemporaryDirectory() as temp_dir:
> +        c_file_name = os.path.join(temp_dir, 'test.c')
> +        obj_file_name = os.path.join(temp_dir, 'test.o')
> +        with open(c_file_name, 'w') as c_file:
> +            c_file.write(snippet)

Sorry, one more thing: Please write '\n' at the end because “snippet”
sometimes does not have a newline at the end (which can trigger warnings
in some cases, IIRC).

Thanks,
Florian

Re: [PATCH v2 2/5] linux: Use compile_c_snippet to check linux/pidfd.h availability

[Florian Weimer]

* Adhemerval Zanella:

> Instead of tying to a specific kernel version.
>
> Checked on x86_64-linux-gnu.
> ---
>  sysdeps/unix/sysv/linux/tst-pidfd-consts.py | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/tst-pidfd-consts.py b/sysdeps/unix/sysv/linux/tst-pidfd-consts.py
> index e207b55eef..d732173abd 100644
> --- a/sysdeps/unix/sysv/linux/tst-pidfd-consts.py
> +++ b/sysdeps/unix/sysv/linux/tst-pidfd-consts.py
> @@ -33,10 +33,12 @@ def main():
>                          help='C compiler (including options) to use')
>      args = parser.parse_args()
>  
> -    linux_version_headers = glibcsyscalls.linux_kernel_version(args.cc)
> -    # Linux started to provide pidfd.h with 5.10.
> -    if linux_version_headers < (5, 10):
> +    if glibcextract.compile_c_snippet(
> +            '#include <linux/pidfd.h>',
> +            args.cc).returncode != 0:
>          sys.exit (77)
> +
> +    linux_version_headers = glibcsyscalls.linux_kernel_version(args.cc)
>      linux_version_glibc = (5, 19)
>      sys.exit(glibcextract.compare_macro_consts(
>                  '#include <sys/pidfd.h>\n',

Looks fine once compile_c_snippet is there.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian

Re: [PATCH v2 1/5] glibcextract.py: Add compile_c_snippet

[Adhemerval Zanella Netto]

On 10/08/22 14:35, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> It might be used on tests to check if a snippet build with the provided
>> compiler and flags.
>> ---
>>  scripts/glibcextract.py | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/scripts/glibcextract.py b/scripts/glibcextract.py
>> index 43ab58ffe2..71d9ea1acc 100644
>> --- a/scripts/glibcextract.py
>> +++ b/scripts/glibcextract.py
>> @@ -21,6 +21,7 @@ import os.path
>>  import re
>>  import subprocess
>>  import tempfile
>> +import collections
> 
> Please keep the imports sorted.

Ack.

> 
>>  def compute_c_consts(sym_data, cc):
>> @@ -173,3 +174,22 @@ def compare_macro_consts(source_1, source_2, cc, macro_re, exclude_re=None,
>>              if not allow_extra_2:
>>                  ret = 1
>>      return ret
>> +
>> +CompileResult = collections.namedtuple("CompileResult", "returncode output")
>> +
>> +def compile_c_snippet(snippet, cc, extra_cc_args=""):
>> +    """Compile and return whether the SNIPPET can be build with CC along
>> +       EXTRA_CC_ARGS compiler flags.  Return a CompileResult with RETURNCODE
>> +       being 0 for success, or the failure value and the compiler output.
>> +    """
>> +    with tempfile.TemporaryDirectory() as temp_dir:
>> +        c_file_name = os.path.join(temp_dir, 'test.c')
>> +        obj_file_name = os.path.join(temp_dir, 'test.o')
>> +        with open(c_file_name, 'w') as c_file:
>> +            c_file.write(snippet)
>> +        # Compilation has to be from stdin to avoid the temporary file
>> +        # name being written into the generated dependencies.
>> +        cmd = ('%s %s -c -o %s -x c - < %s' % (cc, extra_cc_args,
>> +            obj_file_name, c_file_name))
>> +        r = subprocess.run(cmd, shell=True, capture_output=True)
>> +        return CompileResult(r.returncode, r.stderr)
> 
> I think you can avoid shell=True and the direction if you just compile
> c_file_name directly.  Sorry, missed that earlier.

Right, I will do it.

Re: [PATCH v2 5/5] linux: Fix sys/mount.h usage with kernel headers

[Florian Weimer]

* Adhemerval Zanella via Libc-alpha:

> Now that kernel exports linux/mount.h and includes it on linux/fs.h,
> its definitions might clash with glibc exports sys/mount.h.  To avoid
> the need to rearrange the Linux header to be always after glibc one,
> the glibc sys/mount.h is changed to:
>
>   1. Undefine the macros also used as enum constants.  This covers prior
>      inclusion of <linux/mount.h> (for instance MS_RDONLY).
>
>   2. Include <linux/mount.h> based on the usual __has_include check
>      (needs to use __has_include ("linux/mount.h") to paper over GCC
>      bugs.
>
>   3. Define enum fsconfig_command only if FSOPEN_CLOEXEC is not defined.
>      (FSOPEN_CLOEXEC should be a very close proxy.)
>
>   4. Define struct mount_attr if MOUNT_ATTR_SIZE_VER0 is not defined.
>      (Added in the same commit on the Linux side.)
>
> This patch also adds some tests to check if including linux/fs.h and
> linux/mount.h after and before sys/mount.h does work.
>
> Checked on x86_64-linux-gnu.

Also built with build-many-glibcs.py.  This version looks good.

There is a gap with old compilers only (those that don't have
__has_include), and if <linux/mount.h> is included after <sys/mount.h>,
but I think that's an acceptable trade-off.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian

Re: [PATCH v2 5/5] linux: Fix sys/mount.h usage with kernel headers

[Adhemerval Zanella Netto]

On 11/08/22 18:57, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> Now that kernel exports linux/mount.h and includes it on linux/fs.h,
>> its definitions might clash with glibc exports sys/mount.h.  To avoid
>> the need to rearrange the Linux header to be always after glibc one,
>> the glibc sys/mount.h is changed to:
>>
>>   1. Undefine the macros also used as enum constants.  This covers prior
>>      inclusion of <linux/mount.h> (for instance MS_RDONLY).
>>
>>   2. Include <linux/mount.h> based on the usual __has_include check
>>      (needs to use __has_include ("linux/mount.h") to paper over GCC
>>      bugs.
>>
>>   3. Define enum fsconfig_command only if FSOPEN_CLOEXEC is not defined.
>>      (FSOPEN_CLOEXEC should be a very close proxy.)
>>
>>   4. Define struct mount_attr if MOUNT_ATTR_SIZE_VER0 is not defined.
>>      (Added in the same commit on the Linux side.)
>>
>> This patch also adds some tests to check if including linux/fs.h and
>> linux/mount.h after and before sys/mount.h does work.
>>
>> Checked on x86_64-linux-gnu.
> 
> Also built with build-many-glibcs.py.  This version looks good.
> 
> There is a gap with old compilers only (those that don't have
> __has_include), and if <linux/mount.h> is included after <sys/mount.h>,
> but I think that's an acceptable trade-off.

At least gcc 6 does have support for __has_include (or it was backported in
the release branches) which the minimum supported version to build glibc
itself.  I am not sure how common is deployment with gcc version older
with newer glibc, so I agree with the trade-off.

> 
> Reviewed-by: Florian Weimer <fweimer@redhat.com>
> 

Thanks.

> Thanks,
> Florian
>