It wasn't as great idea as I thought. Thing around stat() are more complicated than that. Therefore we need to revert 86d1705a8ace349b plus drop use of the macro as introduced in later patches. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- tests/vircgroupmock.c | 14 +++++++------- tests/virmock.h | 23 ----------------------- tests/virpcimock.c | 19 ++++++++++--------- tests/virtestmock.c | 9 +++++---- 4 files changed, 22 insertions(+), 43 deletions(-) diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index c2d3b80..ce6fd46 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -558,7 +558,7 @@ int __lxstat(int ver, const char *path, struct stat *sb) errno = ENOMEM; return -1; } - ret = VIR_MOCK_CALL_LSTAT(ver, newpath, sb); + ret = real___lxstat(ver, newpath, sb); free(newpath); } else if (STRPREFIX(path, fakedevicedir0)) { sb->st_mode = S_IFBLK; @@ -569,7 +569,7 @@ int __lxstat(int ver, const char *path, struct stat *sb) sb->st_rdev = makedev(9, 0); return 0; } else { - ret = VIR_MOCK_CALL_LSTAT(ver, path, sb); + ret = real___lxstat(ver, path, sb); } return ret; } @@ -589,7 +589,7 @@ int lstat(const char *path, struct stat *sb) errno = ENOMEM; return -1; } - ret = VIR_MOCK_CALL_LSTAT(_STAT_VER, newpath, sb); + ret = real_lstat(newpath, sb); free(newpath); } else if (STRPREFIX(path, fakedevicedir0)) { sb->st_mode = S_IFBLK; @@ -600,7 +600,7 @@ int lstat(const char *path, struct stat *sb) sb->st_rdev = makedev(9, 0); return 0; } else { - ret = VIR_MOCK_CALL_LSTAT(_STAT_VER, path, sb); + ret = real_lstat(path, sb); } return ret; } @@ -620,7 +620,7 @@ int __xstat(int ver, const char *path, struct stat *sb) errno = ENOMEM; return -1; } - ret = VIR_MOCK_CALL_STAT(ver, newpath, sb); + ret = real___xstat(ver, newpath, sb); free(newpath); } else if (STRPREFIX(path, fakedevicedir0)) { sb->st_mode = S_IFBLK; @@ -631,7 +631,7 @@ int __xstat(int ver, const char *path, struct stat *sb) sb->st_rdev = makedev(9, 0); return 0; } else { - ret = VIR_MOCK_CALL_STAT(ver, path, sb); + ret = real___xstat(ver, path, sb); } return ret; } @@ -671,7 +671,7 @@ int stat(const char *path, struct stat *sb) if (!(newpath = strdup(path))) return -1; } - ret = VIR_MOCK_CALL_STAT(_STAT_VER, newpath, sb); + ret = real_stat(newpath, sb); free(newpath); return ret; } diff --git a/tests/virmock.h b/tests/virmock.h index 27c03ba..62a7c8f 100644 --- a/tests/virmock.h +++ b/tests/virmock.h @@ -27,7 +27,6 @@ # endif # include <stdlib.h> # include <stdio.h> -# include <sys/stat.h> # include "internal.h" @@ -255,28 +254,6 @@ static void (*real_##name)(void); \ void name(void) -static inline int -callStat(int (*realStat)(const char *, struct stat *), - int (*realXstat)(int, const char *, struct stat *), - int __ver, const char *__filename, struct stat *__stat_buf) -{ - if (!realXstat) { - if (__ver == _STAT_VER) { - return realStat(__filename, __stat_buf); - } else { - fprintf(stderr, "Not handled __xstat(ver=%d)", __ver); - abort(); - } - } - - return realXstat(__ver, __filename, __stat_buf); -} - -# define VIR_MOCK_CALL_STAT(__ver, __filename, __stat_buf) \ - callStat(real_stat, real___xstat, __ver, __filename, __stat_buf) -# define VIR_MOCK_CALL_LSTAT(__ver, __filename, __stat_buf) \ - callStat(real_lstat, real___lxstat, __ver, __filename, __stat_buf) - /* * The VIR_MOCK_WRAP_NNN_MMM() macros are intended for use in the * individual test suites. The define a stub implementation of diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 60b3b74..e9408aa 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -353,7 +353,8 @@ pci_device_new_from_stub(const struct pciDevice *data) /* If there is a config file for the device within virpcitestdata dir, * symlink it. Otherwise create a dummy config file. */ - if (VIR_MOCK_CALL_STAT(_STAT_VER, configSrc, &sb) == 0) { + if ((real_stat && real_stat(configSrc, &sb) == 0) || + (real___xstat && real___xstat(_STAT_VER, configSrc, &sb) == 0)) { /* On success, copy @configSrc into the destination (a copy, * rather than a symlink, is required since we write into the * file, and parallel VPATH builds must not stomp on the @@ -868,10 +869,10 @@ __lxstat(int ver, const char *path, struct stat *sb) char *newpath; if (getrealpath(&newpath, path) < 0) return -1; - ret = VIR_MOCK_CALL_LSTAT(ver, newpath, sb); + ret = real___lxstat(ver, newpath, sb); VIR_FREE(newpath); } else { - ret = VIR_MOCK_CALL_LSTAT(ver, path, sb); + ret = real___lxstat(ver, path, sb); } return ret; } @@ -887,10 +888,10 @@ lstat(const char *path, struct stat *sb) char *newpath; if (getrealpath(&newpath, path) < 0) return -1; - ret = VIR_MOCK_CALL_LSTAT(_STAT_VER, newpath, sb); + ret = real_lstat(newpath, sb); VIR_FREE(newpath); } else { - ret = VIR_MOCK_CALL_LSTAT(_STAT_VER, path, sb); + ret = real_lstat(path, sb); } return ret; } @@ -906,10 +907,10 @@ __xstat(int ver, const char *path, struct stat *sb) char *newpath; if (getrealpath(&newpath, path) < 0) return -1; - ret = VIR_MOCK_CALL_STAT(ver, newpath, sb); + ret = real___xstat(ver, newpath, sb); VIR_FREE(newpath); } else { - ret = VIR_MOCK_CALL_STAT(ver, path, sb); + ret = real___xstat(ver, path, sb); } return ret; } @@ -925,10 +926,10 @@ stat(const char *path, struct stat *sb) char *newpath; if (getrealpath(&newpath, path) < 0) return -1; - ret = VIR_MOCK_CALL_STAT(_STAT_VER, newpath, sb); + ret = real_stat(newpath, sb); VIR_FREE(newpath); } else { - ret = VIR_MOCK_CALL_STAT(_STAT_VER, path, sb); + ret = real_stat(path, sb); } return ret; } diff --git a/tests/virtestmock.c b/tests/virtestmock.c index 59ca5e5..54669ba 100644 --- a/tests/virtestmock.c +++ b/tests/virtestmock.c @@ -26,6 +26,7 @@ #include <fcntl.h> #include <execinfo.h> #include <sys/file.h> +#include <sys/stat.h> #include "internal.h" #include "configmake.h" @@ -192,7 +193,7 @@ int stat(const char *path, struct stat *sb) checkPath(path); - return VIR_MOCK_CALL_STAT(_STAT_VER, path, sb); + return real_stat(path, sb); } int @@ -202,7 +203,7 @@ __xstat(int ver, const char *path, struct stat *sb) checkPath(path); - return VIR_MOCK_CALL_STAT(ver, path, sb); + return real___xstat(ver, path, sb); } int @@ -212,7 +213,7 @@ lstat(const char *path, struct stat *sb) checkPath(path); - return VIR_MOCK_CALL_LSTAT(_STAT_VER, path, sb); + return real_lstat(path, sb); } int @@ -222,5 +223,5 @@ __lxstat(int ver, const char *path, struct stat *sb) checkPath(path); - return VIR_MOCK_CALL_LSTAT(ver, path, sb); + return real___lxstat(ver, path, sb); } -- 2.8.1

There is a lot to explain, but I try to make it as short as possible. I'd start by pasting some parts of sys/stat.h: extern int stat (const char *__restrict __file, struct stat *__restrict __buf) __THROW __nonnull ((1, 2)); extern int __REDIRECT_NTH (stat, (const char *__restrict __file, struct stat *__restrict __buf), stat64) __nonnull ((1, 2)); # define stat stat64 __extern_inline int __NTH (stat (const char *__path, struct stat *__statbuf)) { return __xstat (_STAT_VER, __path, __statbuf); } Only one of these is effective at once, due to some usage of #ifdefs. Nevertheless, it allows you to create a picture about the mess we are dealing with in here. So, basically, while compiling or linking stat() in our code can be transformed into some other func. Or a dragon. Now, if you read stat(2) manpage, esp. "C library/kernel differences" section, you'll learn that glibc uses some tricks for older applications to work. I haven't gotten around actual code that does this, but based on my observations, if 'stat' symbol is found, glibc assumes it's dealing with ancient application. Unfortunately, it can be just ours stat coming from our mock. Therefore, calling stat() from a test will end up in our mock. But since glibc is not exposing the symbol anymore, our call of real_stat() will SIGSEGV immediately as the pointer to function is NULL. Therefore, we should expose only those symbols we know glibc has. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- cfg.mk | 3 ++ configure.ac | 1 + tests/virtestmock.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+) diff --git a/cfg.mk b/cfg.mk index c0aba57..c19f615 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1296,3 +1296,6 @@ exclude_file_name_regexp--sc_prohibit_not_strneq = \ exclude_file_name_regexp--sc_prohibit_dt_without_code = \ ^docs/(newapi\.xsl|(apps|contact)\.html\.in)$$ + +exclude_file_name_regexp--sc_prohibit_always-defined_macros = \ + ^tests/virtestmock.c$$ diff --git a/configure.ac b/configure.ac index 378069d..7eb9847 100644 --- a/configure.ac +++ b/configure.ac @@ -332,6 +332,7 @@ AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \ libtasn1.h sys/ucred.h sys/mount.h]) dnl Check whether endian provides handy macros. AC_CHECK_DECLS([htole64], [], [], [[#include <endian.h>]]) +AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat __lxstat64]) dnl We need to decide at configure time if libvirt will use real atomic dnl operations ("lock free") or emulated ones with a mutex. diff --git a/tests/virtestmock.c b/tests/virtestmock.c index 54669ba..8c3c0b8 100644 --- a/tests/virtestmock.c +++ b/tests/virtestmock.c @@ -34,13 +34,33 @@ #include "viralloc.h" #include "virfile.h" +/* stat can be a macro as follows: + * + * #define stat stat64 + * + * This wouldn't fly with our mock. Make sure that the macro (and + * all its friends) are undefined. We don't want anybody mangling + * our code. */ +#undef stat +#undef stat64 +#undef __xstat +#undef __xstat64 +#undef lstat +#undef lstat64 +#undef __lxstat +#undef __lxstat64 + static int (*real_open)(const char *path, int flags, ...); static FILE *(*real_fopen)(const char *path, const char *mode); static int (*real_access)(const char *path, int mode); static int (*real_stat)(const char *path, struct stat *sb); +static int (*real_stat64)(const char *path, struct stat64 *sb); static int (*real___xstat)(int ver, const char *path, struct stat *sb); +static int (*real___xstat64)(int ver, const char *path, struct stat64 *sb); static int (*real_lstat)(const char *path, struct stat *sb); +static int (*real_lstat64)(const char *path, struct stat64 *sb); static int (*real___lxstat)(int ver, const char *path, struct stat *sb); +static int (*real___lxstat64)(int ver, const char *path, struct stat64 *sb); static const char *progname; const char *output; @@ -56,7 +76,9 @@ static void init_syms(void) VIR_MOCK_REAL_INIT(fopen); VIR_MOCK_REAL_INIT(access); VIR_MOCK_REAL_INIT_ALT(stat, __xstat); + VIR_MOCK_REAL_INIT_ALT(stat64, __xstat64); VIR_MOCK_REAL_INIT_ALT(lstat, __lxstat); + VIR_MOCK_REAL_INIT_ALT(lstat64, __lxstat64); } static void @@ -187,6 +209,27 @@ int access(const char *path, int mode) return real_access(path, mode); } +/* Okay, the following ifdef rain forest may look messy at a + * first glance. But here's the thing: during run time linking of + * a binary, stat() may not be acutally needing symbol stat. It + * might as well not had been stat() in the first place (see the + * reasoning at the beginning of this file). However, if we would + * expose stat symbol here, we will poison the well and trick + * dynamic linker into thinking we are some old binary that still + * uses the symbol. So whenever code from upper layers calls + * stat(), the control would get here, but real_stat can actually + * be a NULL pointer because newer glibc have __xstat instead. + * Worse, it can have __xstat64 instead __xstat. + * + * Anyway, these ifdefs are there to implement the following + * preference function: + * + * stat < stat64 < __xstat < __xstat64 + * + * It's the same story with lstat. + * Also, I feel sorry for you that you had to read this. + */ +#if defined(HAVE_STAT) && !defined(HAVE___XSTAT) int stat(const char *path, struct stat *sb) { init_syms(); @@ -195,7 +238,20 @@ int stat(const char *path, struct stat *sb) return real_stat(path, sb); } +#endif +#if defined(HAVE_STAT64) && !defined(HAVE___XSTAT64) +int stat64(const char *path, struct stat64 *sb) +{ + init_syms(); + + checkPath(path); + + return real_stat64(path, sb); +} +#endif + +#if defined(HAVE___XSTAT) && !defined(HAVE___XSTAT64) int __xstat(int ver, const char *path, struct stat *sb) { @@ -205,7 +261,21 @@ __xstat(int ver, const char *path, struct stat *sb) return real___xstat(ver, path, sb); } +#endif +#if defined(HAVE___XSTAT64) +int +__xstat64(int ver, const char *path, struct stat64 *sb) +{ + init_syms(); + + checkPath(path); + + return real___xstat64(ver, path, sb); +} +#endif + +#if defined(HAVE_LSTAT) && !defined(HAVE___LXSTAT) int lstat(const char *path, struct stat *sb) { @@ -215,7 +285,21 @@ lstat(const char *path, struct stat *sb) return real_lstat(path, sb); } +#endif +#if defined(HAVE_LSTAT64) && !defined(HAVE___LXSTAT64) +int +lstat64(const char *path, struct stat64 *sb) +{ + init_syms(); + + checkPath(path); + + return real_lstat64(path, sb); +} +#endif + +#if defined(HAVE___LXSTAT) && !defined(HAVE___LXSTAT64) int __lxstat(int ver, const char *path, struct stat *sb) { @@ -225,3 +309,16 @@ __lxstat(int ver, const char *path, struct stat *sb) return real___lxstat(ver, path, sb); } +#endif + +#if defined(HAVE___LXSTAT64) +int +__lxstat64(int ver, const char *path, struct stat64 *sb) +{ + init_syms(); + + checkPath(path); + + return real___lxstat64(ver, path, sb); +} +#endif -- 2.8.1

I've encountered this error while trying out this feature on some systems: $ VIR_TEST_FILE_ACCESS=1 ./virhashtest \ libvirt.git/tests/.libs/lt-virhashtest: \ symbol lookup error: libvirt.git/tests/.libs/virtestmock.so: \ undefined symbol: libvirt_event_poll_purge_timeout_semaphore Problem is, linking just libvirt_utils to virmock.la is not enough. We might need to link libvirt_probes.lo too. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- tests/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index 238f6da..afd5426 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1166,6 +1166,7 @@ virtestmock_la_CFLAGS = $(AM_CFLAGS) virtestmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) virtestmock_la_LIBADD = \ $(MOCKLIBS_LIBS) \ + $(PROBES_O) \ ../src/libvirt_util.la else ! WITH_LINUX EXTRA_DIST += virusbtest.c virusbmock.c \ -- 2.8.1