Re: [PATCH for-4.15] autoconf: check endian.h include path

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Feb 04, 2021 at 11:32:41AM +0100, Jan Beulich wrote:
> On 04.02.2021 11:21, Roger Pau Monné wrote:
> > On Thu, Feb 04, 2021 at 11:13:43AM +0100, Jan Beulich wrote:
> >> On 04.02.2021 10:59, Roger Pau Monné wrote:
> >>> On Thu, Feb 04, 2021 at 10:46:58AM +0100, Jan Beulich wrote:
> >>>> On 04.02.2021 10:38, Roger Pau Monne wrote:
> >>>>> --- a/tools/configure.ac
> >>>>> +++ b/tools/configure.ac
> >>>>> @@ -74,6 +74,7 @@ m4_include([../m4/ax_compare_version.m4])
> >>>>>  m4_include([../m4/paths.m4])
> >>>>>  m4_include([../m4/systemd.m4])
> >>>>>  m4_include([../m4/golang.m4])
> >>>>> +m4_include([../m4/header.m4])
> >>>>>  
> >>>>>  AX_XEN_EXPAND_CONFIG()
> >>>>>  
> >>>>> @@ -517,4 +518,6 @@ AC_ARG_ENABLE([pvshim],
> >>>>>  ])
> >>>>>  AC_SUBST(pvshim)
> >>>>>  
> >>>>> +AX_FIND_HEADER([INCLUDE_ENDIAN_H], [endian.h sys/endian.h])
> >>>>
> >>>> Instead of a new macro, can't you use AC_CHECK_HEADERS()?
> >>>
> >>> AC_CHECK_HEADERS doesn't do what we want here: it will instead produce
> >>> a HAVE_header-file define for each header on the list that's present,
> >>> and the action-if-found doesn't get passed the path of the found
> >>> header according to the documentation.
> >>>
> >>> Here I want the variable to be set to the include path of the first
> >>> header on the list that's present on the system.
> >>
> >> I was thinking of
> >>
> >> #if defined(HAVE_SYS_ENDIAN_H)
> >> # include <sys/endian.h>
> >> #elif defined(HAVE_ENDIAN_H)
> >> # include <endian.h>
> >> #else
> >> # error ...
> >> #endif
> > 
> > I think having to replicate this logic in all places that include
> > endian.h is cumbersome.
> 
> Right - I would further encapsulate this in a local header.

IMO encapsulating in configure achieves the same purpose.

> >>>> I'm also not certain about the order of checks - what if both
> >>>> exist?
> >>>
> >>> With my macro the first one will be picked.
> >>
> >> And which one is to be the first one? IOW how likely is it that
> >> on a system having both the first one is what we're after vs
> >> the second one?
> > 
> > Not sure, but the same will happen with your proposal above: in your
> > chunk sys/endian.h will be picked over endian.h.
> 
> Oh, sure - the two points are entirely orthogonal. And I'm
> also not certain at all whether checking sys/ first is
> better, equal, or worse. I simply don't know what the
> conventions are.

I'm not sure either. For the specific case of endian.h I would
expect only one to be present, and I think we should first check for
top level (ie: endian.h) before checking for subfolders (ie: sys/), as
top level should have precedence.

I really don't have a strong opinion either way, so if there's an
argument to do it the other way around that would also be fine.

> As a result I wonder whether we shouldn't
> check that the header provides what we need.

Right, that would be a step forward I think. I'm not opposed to it,
but I also don't have plans to implement myself. Just checking the
path seem to be fine for the purpose here.

It could be expanded to also use AC_CHECK_DECLS to check for specific
declarations once the header path is found.

Thanks, Roger.