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

[Adhemerval Zanella Netto]

On 10/08/22 06:23, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> diff --git a/scripts/glibccompile.py b/scripts/glibccompile.py
>> new file mode 100755
>> index 0000000000..3eb4e30cc4
>> --- /dev/null
>> +++ b/scripts/glibccompile.py
>> @@ -0,0 +1,45 @@
>> +#!/usr/bin/python3
>> +# Provide functions to build code snippets.
>> +# 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 os.path
>> +import subprocess
>> +import tempfile
>> +import collections
>> +
>> +CompileResult = collections.namedtuple("CompileResult", "returncode output")
> 
> Convention is to use '' strings (outside doctext).

Ack, I will change it.

> 
>> +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))
>> +        try:
>> +            return CompileResult(subprocess.run(cmd, shell=True,
>> +                capture_output=True, check=True).returncode, "")
>> +        except subprocess.SubprocessError as e:
>> +            return CompileResult(1, e.stderr)
> 
> It's a bit weird to use check=True and catch the exception.

Right, I think we can simplify to avoid the check=True and the exception
handling.

> 
> Overall, this could probably go into glibcextract, and as a separate patch?

Alright, this sound feasible.

> 
>> diff --git a/sysdeps/unix/sysv/linux/sys/mount.h b/sysdeps/unix/sysv/linux/sys/mount.h
>> index f965986ba8..ae8406838b 100644
>> --- a/sysdeps/unix/sysv/linux/sys/mount.h
>> +++ b/sysdeps/unix/sysv/linux/sys/mount.h
>> @@ -27,6 +27,19 @@
>>  #include <stddef.h>
>>  #include <sys/ioctl.h>
>>  
>> +#ifdef __has_include
>> +# if __has_include ("linux/mount.h")
>> +#  include "linux/mount.h"
>> +#  ifdef FSOPEN_CLOEXEC
>> +#   define __mount_fsopen_cloexec_defined 1
>> +#  endif
>> +#  ifdef MOUNT_ATTR_SIZE_VER0
>> +#   define __mount_attr_size_ver0_defined 1
>> +#  endif
>> +# endif
>> +#endif
> 
> I think those __mount_ defines are not *that* useful.

Not sure I am following, should I remove them? If so it would require some more
ifdefs on their usage below.

> 
>> +
>> +#undef BLOCK_SIZE
>>  #define BLOCK_SIZE	1024
> 
> If we don't have enum constants, should we #undef things?  If we
> redefine as the same token as the kernel, there won't be a preprocessor
> warning.

This is required because of linux/fs.h (I caught it with the new testcase).
Not sure if this is really required now that kernel is avoid including
linux/mount.h on linux/fs.h.