[PATCH] linux: Fix sys/mount.h usage with kernel headers
Adhemerval Zanella Netto
adhemerval.zanella@linaro.org
Wed Aug 10 14:57:14 GMT 2022
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.
More information about the Libc-alpha
mailing list