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). _______________________________________________ netcf-devel mailing list netcf-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/netcf-devel