[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

On Fri, Mar 18, 2016 at 04:18:15PM +0100, Michal Privoznik wrote:
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 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 {
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!

I would say just take all of such cleanups and put them together to one
commit, but others will tell you not to do that.  We should come up with
some universal works-for-all way to do this.

Attachment: signature.asc
Description: Digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]