[libvirt] [PATCH v5 6/9] nss: Implement _nss_libvirt_gethostbyname3_r

[Michal Privoznik]

On 18.03.2016 15:10, Martin Kletzander wrote:
> On Tue, Mar 15, 2016 at 06:05:53PM +0100, Michal Privoznik wrote:
>> The implementation is pretty straightforward. Moreover, because
>> of the nature of things, gethostbyname_r and gethostbyname2_r can
>> be implemented at the same time too.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>> config-post.h              |  24 ++++
>> src/Makefile.am            |  57 ++++++++
>> src/util/virfile.c         |   3 +-
>> src/util/virfile.h         |  10 +-
>> src/util/virlease.c        |   1 +
>> tests/Makefile.am          |   2 +-
>> tools/Makefile.am          |   5 +
>> tools/nss/libvirt_nss.c    | 336
>> ++++++++++++++++++++++++++++++++++++++++++++-
>> tools/nss/libvirt_nss.h    |  14 +-
>> tools/nss/libvirt_nss.syms |   4 +-
>> 10 files changed, 447 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/util/virfile.h b/src/util/virfile.h
>> index 312f226..50a3995 100644
>> --- a/src/util/virfile.h
>> +++ b/src/util/virfile.h
>> @@ -30,7 +30,15 @@
>> # include <dirent.h>
>>
>> # include "internal.h"
>> -# include "virstoragefile.h"
>> +
>> +/* Okay, this is not nice, but we want resulting nss module as
>> + * small as possible. Including virstoragefile.h would drag in
>> + * libxml2 dependencies which is unfavorable. */
>> +# ifdef LIBVIRT_NSS
>> +#  define virStorageFileFormat int
>> +# else
>> +#  include "virstoragefile.h"
>> +# endif
>>
> 
> I agree with Daniel here.  Just add LIBXML2_CFLAGS to needed targets so
> that it compiles properly.  And don't compile in the virstoragefile.c.
> 
>> typedef enum {
>>     VIR_FILE_CLOSE_PRESERVE_ERRNO = 1 << 0,
>> diff --git a/src/util/virlease.c b/src/util/virlease.c
>> index 910c003..920ebaf 100644
>> --- a/src/util/virlease.c
>> +++ b/src/util/virlease.c
>> @@ -30,6 +30,7 @@
>> #include "virstring.h"
>> #include "virerror.h"
>> #include "viralloc.h"
>> +#include "virutil.h"
>>
> 
> This should be in the patch where you introduce virlease.c, I guess.

It was here because after I started including virstoragefile.h
conditionally, I experienced couple of build errors which proven to be
due to a missing include of virutil.h. If we include virstoragefile.h
without any condition, just like it is now, virutil.h is included
indirectly from there as well. Yes, we can be nice and include it here
directly. But truth to be told I'm tired of putting every little change
into its own commit. I can't put this change into 1/9 because then it
wouldn't be just a pure code movement. I need to save it for a separate
patch then. And write a sensible commit message (which will end up being
longer than change itself). D'oh!

> 
>> #define VIR_FROM_THIS VIR_FROM_NETWORK
>>
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index 90981dc..55e8432 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -65,7 +65,7 @@ GNULIB_LIBS = \
>>        ../gnulib/lib/libgnu.la
>>
>> LDADDS = \
>> -        $(WARN_CFLAGS) \
>> +    $(WARN_CFLAGS) \
>>     $(NO_INDIRECT_LDFLAGS) \
>>     $(PROBES_O) \
>>     $(GNULIB_LIBS) \
> 
> Drop this hunk.
> 
> ACK with those nits fixed.

Thanks.

Michal