On 18/01/17 21:42, selva.n...@gmail.com wrote: > From: Selva Nair <selva.n...@gmail.com> > > - Also make tests that require --wrap option to be > conditional on this support > > Signed-off-by: Selva Nair <selva.n...@gmail.com> > --- > configure.ac | 26 ++++++++++++++++++++++++++ > tests/unit_tests/openvpn/Makefile.am | 6 +++++- > 2 files changed, 31 insertions(+), 1 deletion(-)
This looks like a very good approach to me, and I believe it is done correctly. One comment though, below ... > diff --git a/configure.ac b/configure.ac > index 43487b0..0b7fea9 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -582,6 +582,31 @@ AC_COMPILE_IFELSE( [...snip...] > + AC_DEFINE([HAVE_LD_WRAP_SUPPORT], [1], [Define to 1 if linker > supports -Wl,--wrap option]) Any reason to have this AC_DEFINE? That puts HAVE_LD_WRAP_SUPPORT into config.h, which I don't think makes much sense. If we don't have --wrap support, would we have #ifdef HAVE_LD_WRAP_SUPPORT in the C code at all? [...snip...] > +AM_CONDITIONAL([HAVE_LD_WRAP_SUPPORT], [test "${have_ld_wrap_support}" = > "yes"]) This however is the clue of it all, to ensure we can do the proper check in Makefile.am files, as you've done below > +if HAVE_LD_WRAP_SUPPORT > +check_PROGRAMS += argv_testdriver buffer_testdriver > +endif It would be great if macOS/OSX users with LLVM compilers installed could run some tests with this patch by EOB tomorrow (Friday Jan 20). If I don't hear any objects by then, I am going to give this an ACK without the AC_DEFINE line (unless good arguments having this in config.h surfaces). Selva, if you don't mind ... I can use this patch and just take out the AC_DEFINE line at commit time. -- kind regards, David Sommerseth OpenVPN Technologies, Inc
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel