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

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(a)redhat.com&gt; >> --- >> 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!