The AM_CPPFLAGS setting includes the gnulib headers, which
means we can get some replacement functions defined. Since
virt-login-shell and the NSS module intentionally don't link
to gnulib, these replacement functions causes link failures.

This was seen cross-compiling on Debian for example:

virt-login-shell.o: In function `main':
/builds/libvirt/libvirt/build/tools/../../tools/virt-login-shell.c:81: undefined reference to `rpl_strerror'
/builds/libvirt/libvirt/build/tools/../../tools/virt-login-shell.c:66: undefined reference to `rpl_strerror'
/builds/libvirt/libvirt/build/tools/../../tools/virt-login-shell.c:75: undefined reference to `rpl_strerror'

The only way to avoid these replacement gnulib headers is
to drop the -Ignulib/lib flags. We do still want to use
gnulib for configmake.h and intprops.h, but those can be
included via their full path.

We must also stop using internal.h, since that expects
-Ignulib/lib to be on the include path in order to resolve
the verify.h header.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

Technically a CI build fix, but waiting for review before
pushing since it is larger than most build fixes.

 cfg.mk                         |  3 +++
 tools/Makefile.am              | 10 ++++++++++
 tools/nss/libvirt_nss.c        | 13 ++++++++++---
 tools/nss/libvirt_nss_leases.c | 15 ++++++++-------
 tools/nss/libvirt_nss_leases.h |  2 +-
 tools/nss/libvirt_nss_macs.c   |  6 +++---
 tools/nss/libvirt_nss_macs.h   |  2 +-
 tools/virt-login-shell.c       |  8 ++++++--
 8 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index cc1f79a051..f082448293 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1345,3 +1345,6 @@ exclude_file_name_regexp--sc_prohibit_cross_inclusion = \
 
 exclude_file_name_regexp--sc_prohibit_dirent_d_type = \
   ^(src/util/vircgroup.c)$
+
+exclude_file_name_regexp--sc_prohibit_strcmp = \
+  ^(tools/nss/libvirt_nss.*\.c)
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 09cada949b..df3d628bab 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -23,6 +23,12 @@ AM_CPPFLAGS = \
 	-I$(top_srcdir) \
 	$(NULL)
 
+# We do not want to accidentally include stuff from gnulib
+# or the main src/ dir or public API dir. Specific files can
+# still be included via their path relative to the root if
+# needed
+STANDALONE_CPPFLAGS = -I$(top_srcdir)
+
 WARN_CFLAGS += $(STRICT_FRAME_LIMIT_CFLAGS)
 
 AM_CFLAGS = \
@@ -199,6 +205,8 @@ virt_host_validate_CFLAGS = \
 virt_login_shell_SOURCES = \
 		virt-login-shell.c
 
+virt_login_shell_CPPFLAGS = $(STANDALONE_CPPFLAGS)
+
 virt_login_shell_helper_SOURCES = \
 		virt-login-shell-helper.c
 
@@ -486,6 +494,7 @@ noinst_LTLIBRARIES += nss/libnss_libvirt_impl.la
 nss_libnss_libvirt_impl_la_SOURCES = \
 	$(LIBVIRT_NSS_SOURCES)
 
+nss_libnss_libvirt_impl_la_CPPFLAGS = $(STANDALONE_CPPFLAGS)
 nss_libnss_libvirt_impl_la_CFLAGS = \
 	-DLIBVIRT_NSS \
 	$(YAJL_CFLAGS) \
@@ -516,6 +525,7 @@ nss_libnss_libvirt_guest_impl_la_SOURCES = \
 	nss/libvirt_nss_macs.c \
 	$(NULL)
 
+nss_libnss_libvirt_guest_impl_la_CPPFLAGS = $(STANDALONE_CPPFLAGS)
 nss_libnss_libvirt_guest_impl_la_CFLAGS = \
 	-DLIBVIRT_NSS \
 	-DLIBVIRT_NSS_GUEST \
diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c
index 7122065b28..b75f51c560 100644
--- a/tools/nss/libvirt_nss.c
+++ b/tools/nss/libvirt_nss.c
@@ -32,6 +32,8 @@
 #include <dirent.h>
 #include <arpa/inet.h>
 #include <stdlib.h>
+#include <stdbool.h>
+#include <errno.h>
 #include <string.h>
 #include <time.h>
 
@@ -40,7 +42,12 @@
 # include <nsswitch.h>
 #endif
 
-#include "configmake.h"
+/*
+ * This gnulib files is used for its macros only,
+ * so doesn't introduce a link time dep, which we
+ * must avoid
+ */
+#include "gnulib/lib/configmake.h"
 
 #include "libvirt_nss_leases.h"
 
@@ -131,7 +138,7 @@ findLease(const char *name,
         char *path;
         size_t dlen = strlen(entry->d_name);
 
-        if (dlen >= 7 && STREQ(entry->d_name + dlen - 7, ".status")) {
+        if (dlen >= 7 && !strcmp(entry->d_name + dlen - 7, ".status")) {
             char **tmpLease;
             if (asprintf(&path, "%s/%s", leaseDir, entry->d_name) < 0)
                 goto cleanup;
@@ -142,7 +149,7 @@ findLease(const char *name,
             leaseFiles = tmpLease;
             leaseFiles[nleaseFiles++] = path;
 #if defined(LIBVIRT_NSS_GUEST)
-        } else if (dlen >= 5 && STREQ(entry->d_name + dlen - 5, ".macs")) {
+        } else if (dlen >= 5 && !strcmp(entry->d_name + dlen - 5, ".macs")) {
             if (asprintf(&path, "%s/%s", leaseDir, entry->d_name) < 0)
                 goto cleanup;
 
diff --git a/tools/nss/libvirt_nss_leases.c b/tools/nss/libvirt_nss_leases.c
index 48a54d5841..86881641a9 100644
--- a/tools/nss/libvirt_nss_leases.c
+++ b/tools/nss/libvirt_nss_leases.c
@@ -23,6 +23,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #include <fcntl.h>
 
 #include <yajl/yajl_gen.h>
@@ -60,7 +61,7 @@ typedef struct {
 
 
 static int
-appendAddr(const char *name ATTRIBUTE_UNUSED,
+appendAddr(const char *name __attribute__((unused)),
            leaseAddress **tmpAddress,
            size_t *ntmpAddress,
            const char *ipAddr,
@@ -165,7 +166,7 @@ findLeasesParserInteger(void *ctx,
         return 0;
 
     if (parser->state == FIND_LEASES_STATE_ENTRY) {
-        if (STRNEQ(parser->key, "expiry-time"))
+        if (strcmp(parser->key, "expiry-time"))
             return 0;
 
         parser->entry.expiry = val;
@@ -190,13 +191,13 @@ findLeasesParserString(void *ctx,
         return 0;
 
     if (parser->state == FIND_LEASES_STATE_ENTRY) {
-        if (STREQ(parser->key, "ip-address")) {
+        if (!strcmp(parser->key, "ip-address")) {
             if (!(parser->entry.ipaddr = strndup((char *)stringVal, stringLen)))
                 return 0;
-        } else if (STREQ(parser->key, "mac-address")) {
+        } else if (!strcmp(parser->key, "mac-address")) {
             if (!(parser->entry.macaddr = strndup((char *)stringVal, stringLen)))
                 return 0;
-        } else if (STREQ(parser->key, "hostname")) {
+        } else if (!strcmp(parser->key, "hostname")) {
             if (!(parser->entry.hostname = strndup((char *)stringVal, stringLen)))
                 return 0;
         } else {
@@ -264,12 +265,12 @@ findLeasesParserEndMap(void *ctx)
         DEBUG("Check %zu macs", parser->nmacs);
         for (i = 0; i < parser->nmacs && !found; i++) {
             DEBUG("Check mac '%s' vs '%s'", parser->macs[i], NULLSTR(parser->entry.macaddr));
-            if (STREQ_NULLABLE(parser->macs[i], parser->entry.macaddr))
+            if (parser->entry.macaddr && !strcmp(parser->macs[i], parser->entry.macaddr))
                 found = true;
         }
     } else {
         DEBUG("Check name '%s' vs '%s'", parser->name, NULLSTR(parser->entry.hostname));
-        if (STREQ_NULLABLE(parser->name, parser->entry.hostname))
+        if (parser->entry.hostname && !strcmp(parser->name, parser->entry.hostname))
             found = true;
     }
     DEBUG("Found %d", found);
diff --git a/tools/nss/libvirt_nss_leases.h b/tools/nss/libvirt_nss_leases.h
index e213681e46..c451742152 100644
--- a/tools/nss/libvirt_nss_leases.h
+++ b/tools/nss/libvirt_nss_leases.h
@@ -20,7 +20,7 @@
 
 #pragma once
 
-#include "internal.h"
+#include <sys/types.h>
 
 typedef struct {
     unsigned char addr[16];
diff --git a/tools/nss/libvirt_nss_macs.c b/tools/nss/libvirt_nss_macs.c
index fb5526bd7b..26ba4bf515 100644
--- a/tools/nss/libvirt_nss_macs.c
+++ b/tools/nss/libvirt_nss_macs.c
@@ -67,7 +67,7 @@ findMACsParserString(void *ctx,
         return 0;
 
     if (parser->state == FIND_MACS_STATE_ENTRY) {
-        if (STRNEQ(parser->key, "domain"))
+        if (strcmp(parser->key, "domain"))
             return 0;
 
         free(parser->entry.name);
@@ -75,7 +75,7 @@ findMACsParserString(void *ctx,
             return 0;
     } else if (parser->state == FIND_MACS_STATE_ENTRY_MACS) {
         char **macs;
-        if (STRNEQ(parser->key, "macs"))
+        if (strcmp(parser->key, "macs"))
             return 0;
 
         if (!(macs = realloc(parser->entry.macs,
@@ -142,7 +142,7 @@ findMACsParserEndMap(void *ctx)
     if (parser->state != FIND_MACS_STATE_ENTRY)
         return 0;
 
-    if (STREQ(parser->entry.name, parser->name)) {
+    if (!strcmp(parser->entry.name, parser->name)) {
         char **macs = realloc(*parser->macs,
                               sizeof(char *) * ((*parser->nmacs) + parser->entry.nmacs));
         if (!macs)
diff --git a/tools/nss/libvirt_nss_macs.h b/tools/nss/libvirt_nss_macs.h
index 64e291f549..2774f3a9f2 100644
--- a/tools/nss/libvirt_nss_macs.h
+++ b/tools/nss/libvirt_nss_macs.h
@@ -20,7 +20,7 @@
 
 #pragma once
 
-#include "internal.h"
+#include <sys/types.h>
 
 int
 findMACs(const char *file,
diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index 41d0b349aa..f92cc0a749 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -28,8 +28,12 @@
 #include <errno.h>
 #include <string.h>
 
-#include "configmake.h"
-#include "intprops.h"
+/*
+ * These gnulib files are used for their macros only,
+ * so don't introduce a link time dep, which we must avoid
+ */
+#include "gnulib/lib/configmake.h"
+#include "gnulib/lib/intprops.h"
 
 int main(int argc, char **argv) {
     char uidstr[INT_BUFSIZE_BOUND(uid_t)];
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 8/8/19 1:00 PM, Daniel P. Berrangé wrote:
> The AM_CPPFLAGS setting includes the gnulib headers, which
> means we can get some replacement functions defined. Since
> virt-login-shell and the NSS module intentionally don't link
> to gnulib, these replacement functions causes link failures.
> 
> This was seen cross-compiling on Debian for example:
> 
> virt-login-shell.o: In function `main':
> /builds/libvirt/libvirt/build/tools/../../tools/virt-login-shell.c:81: undefined reference to `rpl_strerror'
> /builds/libvirt/libvirt/build/tools/../../tools/virt-login-shell.c:66: undefined reference to `rpl_strerror'
> /builds/libvirt/libvirt/build/tools/../../tools/virt-login-shell.c:75: undefined reference to `rpl_strerror'
> 
> The only way to avoid these replacement gnulib headers is
> to drop the -Ignulib/lib flags. We do still want to use
> gnulib for configmake.h and intprops.h, but those can be
> included via their full path.
> 
> We must also stop using internal.h, since that expects
> -Ignulib/lib to be on the include path in order to resolve
> the verify.h header.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> 
> Technically a CI build fix, but waiting for review before
> pushing since it is larger than most build fixes.
> 
>  cfg.mk                         |  3 +++
>  tools/Makefile.am              | 10 ++++++++++
>  tools/nss/libvirt_nss.c        | 13 ++++++++++---
>  tools/nss/libvirt_nss_leases.c | 15 ++++++++-------
>  tools/nss/libvirt_nss_leases.h |  2 +-
>  tools/nss/libvirt_nss_macs.c   |  6 +++---
>  tools/nss/libvirt_nss_macs.h   |  2 +-
>  tools/virt-login-shell.c       |  8 ++++++--
>  8 files changed, 42 insertions(+), 17 deletions(-)

Ooops.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list