Re: [PATCH] build: Use AC_HEADER_MAJOR to find device macros

[Mike Gilbert]

On Sun, Apr 17, 2016 at 1:53 AM, Andrei Borzenkov <address@hidden> wrote:
> 17.04.2016 00:34, Mike Gilbert пишет:
>> Depending on the OS/libc, device macros may be found in 3 places:
>>
>
> Mentioning OS and libc versions that have problem would be helpful.
>

I am really only familiar with glibc, though I believe BSD and Sun use
sys/mkdev.h?

>> sys/types.h
>> sys/mkdev.h
>> sys/sysmacros.h
>>
>> glibc currenctly defines the major/minor/makedev macros in sys/sysmacros.h
>> and includes this from sys/types.h. Based on mailing list discussion,
>> this may be removed from sys/types.h in a future glibc release.
>> ---
>>  configure.ac                         | 3 ++-
>>  grub-core/osdep/devmapper/getroot.c  | 6 ++++++
>>  grub-core/osdep/devmapper/hostdisk.c | 5 +++++
>>  grub-core/osdep/linux/getroot.c      | 6 ++++++
>
> All those files are linux only. Do you mean there are some linux flavors
> that have these definitions in non-standard place? Could you name them?
>

As I mentioned, in a future release, glibc may remove #include
<sys/sysmacros.h> from sys/types.h. This would provide preventing
naming conflicts in programs that have variables named like "major"
and indirectly include sys/types.h. This is currently up for
discussion upstream.

https://sourceware.org/ml/libc-alpha/2015-11/msg00253.html

https://sourceware.org/ml/libc-alpha/2015-12/msg00612.html

If you would prefer to wait until a commit lands in glibc, I
understand. We have been doing some testing in Gentoo to see how many
things will break (it's a lot!). I just thought I would submit this
now since the check is fairly harmless and would future-proof the code
either way.

>>  grub-core/osdep/unix/getroot.c       | 4 +++-
>>  5 files changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 57e1713..9ddfc53 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -388,7 +388,8 @@ fi
>>
>>  # Check for functions and headers.
>>  AC_CHECK_FUNCS(posix_memalign memalign getextmntent)
>> -AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h sys/mkdev.h limits.h)
>> +AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h)
>> +AC_HEADER_MAJOR
>>
>>  AC_CHECK_MEMBERS([struct statfs.f_fstypename],,,[$ac_includes_default
>>  #include <sys/param.h>
>> diff --git a/grub-core/osdep/devmapper/getroot.c 
>> b/grub-core/osdep/devmapper/getroot.c
>> index 05eda50..72e5582 100644
>> --- a/grub-core/osdep/devmapper/getroot.c
>> +++ b/grub-core/osdep/devmapper/getroot.c
>> @@ -40,6 +40,12 @@
>>  #include <limits.h>
>>  #endif
>>
>> +#if defined(MAJOR_IN_MKDEV)
>> +#include <sys/mkdev.h>
>> +#elif defined(MAJOR_IN_SYSMACROS)
>> +#include <sys/sysmacros.h>
>> +#endif
>> +
>>  #include <libdevmapper.h>
>>
>>  #include <grub/types.h>
>> diff --git a/grub-core/osdep/devmapper/hostdisk.c 
>> b/grub-core/osdep/devmapper/hostdisk.c
>> index 19c1101..a697bcb 100644
>> --- a/grub-core/osdep/devmapper/hostdisk.c
>> +++ b/grub-core/osdep/devmapper/hostdisk.c
>> @@ -24,6 +24,11 @@
>>  #include <errno.h>
>>  #include <limits.h>
>>
>> +#if defined(MAJOR_IN_MKDEV)
>> +#include <sys/mkdev.h>
>> +#elif defined(MAJOR_IN_SYSMACROS)
>> +#include <sys/sysmacros.h>
>> +#endif
>>
>>  #ifdef HAVE_DEVICE_MAPPER
>>  # include <libdevmapper.h>
>> diff --git a/grub-core/osdep/linux/getroot.c 
>> b/grub-core/osdep/linux/getroot.c
>> index 10480b6..09e7e6e 100644
>> --- a/grub-core/osdep/linux/getroot.c
>> +++ b/grub-core/osdep/linux/getroot.c
>> @@ -35,6 +35,12 @@
>>  #include <limits.h>
>>  #endif
>>
>> +#if defined(MAJOR_IN_MKDEV)
>> +#include <sys/mkdev.h>
>> +#elif defined(MAJOR_IN_SYSMACROS)
>> +#include <sys/sysmacros.h>
>> +#endif
>> +
>>  #include <grub/types.h>
>>  #include <sys/ioctl.h>         /* ioctl */
>>  #include <sys/mount.h>
>> diff --git a/grub-core/osdep/unix/getroot.c b/grub-core/osdep/unix/getroot.c
>> index 1079a91..4bf37b0 100644
>> --- a/grub-core/osdep/unix/getroot.c
>> +++ b/grub-core/osdep/unix/getroot.c
>> @@ -51,8 +51,10 @@
>>  #endif
>>
>>  #include <sys/types.h>
>> -#if defined(HAVE_SYS_MKDEV_H)
>> +#if defined(MAJOR_IN_MKDEV)
>>  #include <sys/mkdev.h>
>> +#elif defined(MAJOR_IN_SYSMACROS)
>> +#include <sys/sysmacros.h>
>>  #endif
>>
>
> The names are really misleading. All that this macro checks for is
> whether these headers are present, so it is entirely equivalent to
> AC_CHECK_HEADERS([sys/mkdev.h sys/sysmacros.h]). Which returns us to the
> question which systems have sys/sysmacros.h :)
>

The macro has existed for over 20 years. I can't say why it was named that way.