Re: [PATCH 1/2] osdep: Introduce major.h and use it

[Petr Vorel]

Hi Daniel,

> On Wed, Jul 14, 2021 at 08:54:29AM +0200, Petr Vorel wrote:
> > Hi Daniel,

> > > > On Thu, Jul 08, 2021 at 05:55:57PM +0200, Petr Vorel wrote:
> > > > > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > > > > ---
> > > > >  grub-core/osdep/devmapper/getroot.c  |  7 +------
> > > > >  grub-core/osdep/devmapper/hostdisk.c |  7 +------
> > > > >  grub-core/osdep/linux/getroot.c      |  7 +------
> > > > >  grub-core/osdep/unix/getroot.c       |  7 +------
> > > > >  include/grub/osdep/major.h           | 30 
> > > > > ++++++++++++++++++++++++++++
> > > > >  5 files changed, 34 insertions(+), 24 deletions(-)
> > > > >  create mode 100644 include/grub/osdep/major.h

> > > > May I ask you to explain in the commit message why this patch is needed?
> > > It's just a small cleanup. If you're not against it, sure, I can mention 
> > > it in
> > > the commit message.

> I am OK with the cleanup.
thx for ack!

> > Ah, you probably mean to put the description from major.h also to the commit
> > message.

> To some extent. I think it should be phrased a bit better in the commit 
> message.
sure.

> > Kind regards,
> > Petr

> > +/*
> > + * Fix for glibc 2.25 is deprecating the namespace pollution of sys/types.h
> > + * injecting major(), minor(), and makedev() into the compilation 
> > environment.
> > + * See configure.ac.

> It seems to me "See configure.ac." is not relevant in the GRUB source.
> So, probably it should be replaced with some text from glibc configure.ac.

I didn't know how to link AC_HEADER_MAJOR with include/grub/osdep/major.h.
Because in the future when glibc 2.25 is old enough the header will be removed
and AC_HEADER_MAJOR might be left in configure.ac (name is different from
MAJOR_IN_{MKDEV,SYSMACROS}). But I can note this in commit message instead.

Kind regards,
Petr

> Daniel