[PATCH] virmockstathelpers: Adapt to musl-1.2.4

Michal Privoznik posted 1 patch 11 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1a98a153083c8a93a02429cbf544c6aa71dd3006.1683912035.git.mprivozn@redhat.com
tests/virmockstathelpers.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
[PATCH] virmockstathelpers: Adapt to musl-1.2.4
Posted by Michal Privoznik 11 months, 2 weeks ago
With musl-1.2.3: I get the following macros defined (from
$builddir/meson-config.h):

  #define WITH_LSTAT 1
  #define WITH_LSTAT64 1
  #define WITH_LSTAT_DECL 1
  #define WITH_STAT 1
  #define WITH_STAT64 1
  #define WITH_STAT_DECL 1
  #define WITH___LXSTAT 1
  #define WITH___LXSTAT64 1
  #define WITH___XSTAT 1
  #define WITH___XSTAT64 1

which in turn means the virmockstathelpers.c ends up defining:

  MOCK_STAT64
  MOCK_LSTAT64

But with  musl-1.2.4 everything changes and the set of defined
macros gets simplified to:

  #define WITH_LSTAT 1
  #define WITH_LSTAT_DECL 1
  #define WITH_STAT 1
  #define WITH_STAT_DECL 1
  #define WITH___LXSTAT 1
  #define WITH___XSTAT 1

which results in no MOCK_* macros defined in
virmockstathelpers.c, i.e. no stat() mocking, nada. The reason
for this simplification are thess musl commits [1][2] which removed all
64 bit aliases. And that's not what our logic for deciding what
flavor of stat() to mock counted with.

Nevertheless, we do build with Alpine Linux in our CI, so how
come we don't see this problem there? Well, simply because Alpine
Linux maintainers decided to revert the commits [3][4]. But on
distributions that use vanilla musl, this problem can be seen
easily.

1: https://git.musl-libc.org/cgit/musl/commit/?id=246f1c811448f37a44b41cd8df8d0ef9736d95f4
2: https://git.musl-libc.org/cgit/musl/commit/?id=25e6fee27f4a293728dd15b659170e7b9c7db9bc
3: https://git.alpinelinux.org/aports/commit/main/musl?id=6a5563fbb45b3d9d60678d7bbf60dbb312a2d481
4: https://git.alpinelinux.org/aports/commit/main/musl?id=a089bd852f8983623fa85e0f5755a3e25bf53c72

Resolves: https://bugs.gentoo.org/906167
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tests/virmockstathelpers.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tests/virmockstathelpers.c b/tests/virmockstathelpers.c
index 5b1f3b08a7..a02ee04503 100644
--- a/tests/virmockstathelpers.c
+++ b/tests/virmockstathelpers.c
@@ -64,6 +64,10 @@
  * building on Apple Silicon (aarch64) those functions are missing
  * from the header altogether and should not be mocked.
  *
+ * Starting from 1.2.4, the musl library dropped 64-bit aliases, i.e. there's
+ * no stat64() only stat(). BTW: while musl implements __xstat(), it's never
+ * declared. And even the implementation is but a direct call to stat().
+ *
  * With all this in mind the list of functions we have to mock will depend
  * on several factors
  *
@@ -82,7 +86,9 @@
 #if !defined(__APPLE__)
 # if !defined(WITH___XSTAT_DECL)
 #  if defined(WITH_STAT)
-#   if !defined(WITH___XSTAT) && !defined(WITH_STAT64)
+#   if (!defined(WITH___XSTAT) && !defined(WITH_STAT64)) || \
+    (defined(WITH_STAT) && defined(WITH_STAT_DECL) && !defined(WITH_STAT64) && \
+     !defined(WITH___XSTAT64)) /* glibc || musl */
 #    define MOCK_STAT
 #   endif
 #  endif
@@ -99,7 +105,9 @@
 # endif /* WITH___XSTAT_DECL */
 # if !defined(WITH___LXSTAT_DECL)
 #  if defined(WITH_LSTAT)
-#   if !defined(WITH___LXSTAT) && !defined(WITH_LSTAT64)
+#   if (!defined(WITH___LXSTAT) && !defined(WITH_LSTAT64)) || \
+    (defined(WITH_LSTAT) && defined(WITH_LSTAT_DECL) && !defined(WITH_LSTAT64) \
+     && !defined(WITH___LXSTAT64)) /* glibc || musl */
 #    define MOCK_LSTAT
 #   endif
 #  endif
-- 
2.39.3
Re: [PATCH] virmockstathelpers: Adapt to musl-1.2.4
Posted by Michal Prívozník 11 months, 2 weeks ago
On 5/12/23 19:20, Michal Privoznik wrote:
>

And I just thought of another alternative:

diff --git i/tests/virmockstathelpers.c w/tests/virmockstathelpers.c
index 5b1f3b08a7..8a76c5e369 100644
--- i/tests/virmockstathelpers.c
+++ w/tests/virmockstathelpers.c
@@ -118,20 +118,30 @@
 # define MOCK_STAT
 # if defined(WITH_STAT64_DECL)
 #  define MOCK_STAT64
 # endif
 # define MOCK_LSTAT
 # if defined(WITH_LSTAT64_DECL)
 #  define MOCK_LSTAT64
 # endif
 #endif
 
+#if !defined(MOCK_STAT) && !defined(MOCK_STAT64) && \
+    !defined(MOCK___XSTAT) && !defined(MOCK___XSTAT64)
+# define MOCK_STAT
+#endif
+
+#if !defined(MOCK_LSTAT) && !defined(MOCK_LSTAT64) && \
+    !defined(MOCK___LXSTAT) && !defined(MOCK___LXSTAT64)
+# define MOCK_LSTAT
+#endif
+
 #ifdef MOCK_STAT
 static int (*real_stat)(const char *path, struct stat *sb);
 #endif
 #ifdef MOCK_STAT64
 static int (*real_stat64)(const char *path, struct stat64 *sb);
 #endif
 #ifdef MOCK___XSTAT
 static int (*real___xstat)(int ver, const char *path, struct stat *sb);
 #endif
 #ifdef MOCK___XSTAT64


But I can't decide which is better.

Michal
Re: [PATCH] virmockstathelpers: Adapt to musl-1.2.4
Posted by Martin Kletzander 11 months, 2 weeks ago
On Sat, May 13, 2023 at 12:12:41AM +0200, Michal Prívozník wrote:
>On 5/12/23 19:20, Michal Privoznik wrote:
>>
>
>And I just thought of another alternative:
>
>diff --git i/tests/virmockstathelpers.c w/tests/virmockstathelpers.c
>index 5b1f3b08a7..8a76c5e369 100644
>--- i/tests/virmockstathelpers.c
>+++ w/tests/virmockstathelpers.c
>@@ -118,20 +118,30 @@
> # define MOCK_STAT
> # if defined(WITH_STAT64_DECL)
> #  define MOCK_STAT64
> # endif
> # define MOCK_LSTAT
> # if defined(WITH_LSTAT64_DECL)
> #  define MOCK_LSTAT64
> # endif
> #endif
>
>+#if !defined(MOCK_STAT) && !defined(MOCK_STAT64) && \
>+    !defined(MOCK___XSTAT) && !defined(MOCK___XSTAT64)
>+# define MOCK_STAT
>+#endif
>+
>+#if !defined(MOCK_LSTAT) && !defined(MOCK_LSTAT64) && \
>+    !defined(MOCK___LXSTAT) && !defined(MOCK___LXSTAT64)
>+# define MOCK_LSTAT
>+#endif
>+
> #ifdef MOCK_STAT
> static int (*real_stat)(const char *path, struct stat *sb);
> #endif
> #ifdef MOCK_STAT64
> static int (*real_stat64)(const char *path, struct stat64 *sb);
> #endif
> #ifdef MOCK___XSTAT
> static int (*real___xstat)(int ver, const char *path, struct stat *sb);
> #endif
> #ifdef MOCK___XSTAT64
>
>
>But I can't decide which is better.
>

I think this one is better because it basically says "if there's nothing
to mock, then mock stat".  It is more readable, but I wasn't sure
whether it will also work on 32bit musl with largefile64.  After some
mental exercise I think it will.  So this should be safe to push and
I'll setup an environment for it in the meantime and will report results
afterwards.

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>


>Michal
>