This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 1/2] sysdeps/stat: Handle 64-bit ino_t types on 32-bit hosts


On Wed, 18 Sep 2019, Alistair Francis wrote:

> On a 32-bit platform with a 64-bit ino_t type (__INO_T_MATCHES_INO64_T
> defined) we want to update the stat struct to remove the padding as it
> isn't required. As we don't have the padding we also need to update the
> overflow checker to not access the undefined members.

When posting a patch, please *always* say how it was tested.  Typically 
that would include running the glibc testsuite for at least one 
configuration (but there may be exceptions, e.g. documentation patches).

> -#if defined __USE_FILE_OFFSET64
> +#if defined(__USE_FILE_OFFSET64) || __INO_T_MATCHES_INO64_T == 1
> +# if __INO_T_MATCHES_INO64_T == 1 && __OFF_T_MATCHES_OFF64_T != 1

The rule in glibc is that we compile with -Wundef and do not rely on 
undefined macros being interpreted as 0 in #if.

Both __INO_T_MATCHES_INO64_T and __OFF_T_MATCHES_OFF64_T are in fact 
defined/undefined macros, not macros that might sometimes be defined to 1 
and sometimes to 0.  So testing with == there is problematic; I'd expect 
it to cause -Wundef errors for any existing 32-bit linux/generic 
architecture where those macros are undefined (i.e. csky and nios2).

There are two possible approaches to fixing this.

(a) Do a preliminary patch that changes these macros from defined / 
undefined to always-defined with 1 / 0 values.  This involves updating a 
lot of places that use the macros (mainly using __OFF_T_MATCHES_OFF64_T), 
as well as ensuring that all bits/typesizes.h headers provide appropriate 
definitions.

(b) Change this patch to do defined / undefined tests rather than testing 
the macro values.

Also, when a macro is 0 / 1 I'd expect boolean tests just to test the 
macro as a boolean (so plain "__INO_T_MATCHES_INO64_T" inside #if, not 
"__INO_T_MATCHES_INO64_T == 1").

Also, if using "defined" with (), like a function call there should be a 
space before '('.

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]