[netcf-devel] [PATCH] fix if.h problems when compiling with newer libnl3

Laine Stump laine at laine.org
Wed Apr 8 14:47:20 UTC 2015


On 04/08/2015 09:48 AM, Laine Stump wrote:
> On 04/07/2015 11:47 PM, Eric Blake wrote:
>> On 04/07/2015 04:20 PM, Laine Stump wrote:
>>> When netcf first include support for libnl3, the author of that patch
>>> (Serge Hallyn, commit 0310cd505) found that
>>> /usr/include/libnl3/netlink/route/link.h #included <linux/if.h>, which
>>> conflicts with <net/if.h> (but for our purposes at least defines all the
>>> same things). So he put the #include <net/if.h> in dutil_linux.c inside
>>> an #ifndef HAVE_LIBNL3.
>>>
>>> That worked fine until libnl3 finally removed the offending (and
>>> unnecessary) #include <linux/if.h> from its link.h. That fix to libnl3
>>> broke the netcf build.
>>>
>>> This patch fixes the build by using autotools magic to attempt
>>> compiling essentially this short program:
>>>
>>>   #include <netlink/route/link.h>
>>>   int main()
>>>   {
>>>       int try = IFF_UP;
>>>       struct ifreq ifr;
>>>       return 0;
>> If someone configures with high levels of warnings enabled, this might
>> spuriously fail due to unused variables.  Something like:
>>
>>   struct ifreq ifr;
>>   return ifr.ifr_ivalue = IFF_UP;
>>
>> is slightly stronger (nothing goes unused), even if weird (no one would
>> actually write this anywhere besides configure.ac).  Then again, the
>> autoconf list frequently tells people that run './configure
>> CFLAGS=-Werror' to not do it - there is NO way to write warning-free
>> code for every possible compiler version, especially when probing for
>> features; and therefore worrying about compiler warnings in configure
>> snippets is not worth the effort.  So I would not bother rewriting the
>> code _unless_ someone complains that it has a false failure during
>> configure, which I doubt will happen.
> Still, I like the idea of creating less warnings to confuse the
> config.log output. I'll change it as you suggest before pushing.
>
>>>   }
>>>
>>> If this can compile successfully without any other includes, then
>>> link.h is already including linux/if.h and we should avoid including
>>> net/if.h, so we #define AVOID_NET_IF_H. If the compile fails, then we
>>> do need to include net/if.h, so we *don't* #define
>>> AVOID_NET_IF_H. Then we enclose dutil_linux.c's #include <net/if.h>
>>> within #ifndef AVOID_NET_IF_H and we're done.
>>>
>>> Although this check would probably be safe on a libnl1 system, we know
>>> from past experience that we should always include net/if.h in those
>>> cases anyway, so we don't even try - we only do the configure-time
>>> check if we're using libnl3.
>>> ---
>>>  configure.ac      | 19 ++++++++++++++++++-
>>>  src/dutil_linux.c |  4 ++--
>>>  2 files changed, 20 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index a914b0f..6edbb5e 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -164,7 +164,24 @@ then
>>>  	if (test "${have_libnl3}" = "yes" -a "${have_libnl_route3}" = "yes"); then
>>>  		AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0])
>>>  		have_libnl="yes"
>>> -	else
>>> +                SAVE_CFLAGS="${CFLAGS}"
>> Difference in TAB vs. space (not fatal, but you may want to be consistent)
> My emacs is setup to pretty much always insert spaces instead of tabs.
> It looks like a few committers in the past have used tabs, though. Since
> the file is already mixed (mostly spaces, though) and I don't want to
> create a bunch of merge conflict opportunities, I'm going to leave my
> additions using spaces.
>
>
>>> +                CFLAGS="${LIBNL_CFLAGS} ${CFLAGS}"
>>> +                dnl Avoid #including net/if.h when using earlier versions
>>> +                dnl of libnl3 that include linux/if.h from netlink/route/link.h,
>>> +                dnl since the two if.h files conflict with each other.
>>> +                AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
>>> +                    [[
>>> +                      #include <netlink/route/link.h>
>>> +                    ]],
>>> +                    [[struct ifreq ifr; int try = IFF_UP;]])],
>>> +                    [avoid_net_if_h=yes], [avoid_net_if_h=no])
>> Looks correct.  The only improvement might be to wrap it all in
>> AC_CACHE_CHECK and rename the witness variable to something like
>> netcf_cv_avoid_net_if_h, so that it can be primed through a configure
>> cache rather than probed every time. Again, not sure it would matter
>> until someone actually complains that it guessed wrong and they wished
>> for a way to prime the cache to avoid the wrong guess.
> Since I don't understand AC_CACHE_CHECK and want to get this in fairly
> quickly, I think I won't try to add it.

I just decided to go the other way on this, not for the caching and
ability to override the guess made by the configure script, but because
AC_CACHE_CHECK prints out a nifty message to the console (and because it
was incredibly easy to do - just change the name as you suggest, and put
the AC_COMPILE_IFELSE() inside AC_CACHE_CHECK (along with a couple other
arguments).



More information about the netcf-devel mailing list