On 11/2/25 14:13, Pádraig Brady wrote:
On 01/11/2025 23:49, Bernhard Voelker wrote:
Thanks for verifying.
But before pushing this second part, I'd rather like to understand why gnulib's
read_file_system_list now uses fopen instead of open again ... because this 
would
contradicts Padraig's commit 6f1ec80f31164e from last year.
I suspect gnulib is now using openat() rather than open().
If so we could adjust to wrap openat or fopen.
Since fopen is not replaced for me on glibc 2.41 (when it was on 2.39),
let's assume it's not replaced on current systems and go with both of your 
patches.

That would explain it, I agree.
The attached patch #2 wraps both open and fopen, and performs the test here in 
the
fopen code path successfully.

  $ make check SUBDIRS=. TESTS="tests/df/no-mtab-status.sh 
tests/df/skip-duplicates.sh"
  ...
  PASS: tests/df/no-mtab-status.sh
  PASS: tests/df/skip-duplicates.sh

@Padraig: does it work for you with open as well?
Okay to push both?

Have a nice day,
Berny
From 98d2cd6db73ce51559dd3e40153a3d8a8f3ba2e3 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <[email protected]>
Date: Sat, 1 Nov 2025 20:32:21 +0100
Subject: [PATCH 1/2] tests: avoid skipping by fixing build of shared libraries

Two df(1) tests were skipped (since commit ee367bd38dac), because
the build of the shared library in those tests failed.

  + gcc -Wall -shared --std=gnu99 -fPIC -O2 k.c -o k.so -ldl
  k.c: In function 'open':
  k.c:37:7: error: implicit declaration of function 'streq'; did you \
  mean 'strsep'? [-Wimplicit-function-declaration]
     37 |   if (streq (path, "/proc/self/mountinfo"))
        |       ^~~~~
        |       strsep

Gnulib streq is not available in the tests.

* tests/df/no-mtab-status.sh: Replace "streq" by "0==strcmp" in the
shared library source.
* tests/df/skip-duplicates.sh: Likewise.
---
 tests/df/no-mtab-status.sh  | 3 ++-
 tests/df/skip-duplicates.sh | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/df/no-mtab-status.sh b/tests/df/no-mtab-status.sh
index 0c2e8982d..135462393 100755
--- a/tests/df/no-mtab-status.sh
+++ b/tests/df/no-mtab-status.sh
@@ -28,7 +28,8 @@ grep '^#define HAVE_GETMNTENT 1' $CONFIG_HEADER > /dev/null \
       || skip_ "getmntent is not used on this system"
 
 # Simulate "mtab" failure.
-cat > k.c <<EOF || framework_failure_
+# Replace gnulib streq as that is not available here.
+sed 's/streq/0==str''cmp/' > k.c <<EOF || framework_failure_
 #define _GNU_SOURCE
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/tests/df/skip-duplicates.sh b/tests/df/skip-duplicates.sh
index f97f794f5..c839f4e19 100755
--- a/tests/df/skip-duplicates.sh
+++ b/tests/df/skip-duplicates.sh
@@ -38,7 +38,8 @@ grep '^#define HAVE_GETMNTENT 1' $CONFIG_HEADER > /dev/null \
       || skip_ "getmntent is not used on this system"
 
 # Simulate an mtab file to test various cases.
-cat > k.c <<EOF || framework_failure_
+# Replace gnulib streq as that is not available here.
+sed 's/streq/0==str''cmp/' > k.c <<EOF || framework_failure_
 #define _GNU_SOURCE
 #include <stdio.h>
 #include <stdlib.h>
-- 
2.51.1

From dca6eb2a30c9bfe08a56c53c0fcd607b63150735 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <[email protected]>
Date: Sat, 1 Nov 2025 22:27:59 +0100
Subject: [PATCH 2/2] tests: avoid skipping of LD_PRELOAD based df tests

It was seen that gnulib's read_file_system_list may use fopen instead
of open.  Adjust the df(1) tests to replace both library functions.

* tests/df/no-mtab-status.sh: Change the shared library code invoked
via LD_PRELOAD to override both fopen and open.  While at it, perform
varargs processing only when path is not "/proc/self/mountinfo".
* tests/df/skip-duplicates.sh: Likewise.
---
 tests/df/no-mtab-status.sh  | 57 ++++++++++++++++++++++++++++---------
 tests/df/skip-duplicates.sh | 53 ++++++++++++++++++++++++++--------
 2 files changed, 84 insertions(+), 26 deletions(-)

diff --git a/tests/df/no-mtab-status.sh b/tests/df/no-mtab-status.sh
index 135462393..021e94991 100755
--- a/tests/df/no-mtab-status.sh
+++ b/tests/df/no-mtab-status.sh
@@ -28,8 +28,8 @@ grep '^#define HAVE_GETMNTENT 1' $CONFIG_HEADER > /dev/null \
       || skip_ "getmntent is not used on this system"
 
 # Simulate "mtab" failure.
-# Replace gnulib streq as that is not available here.
-sed 's/streq/0==str''cmp/' > k.c <<EOF || framework_failure_
+# Replace gnulib streq and C23 nullptr as that are not available here.
+sed 's/streq/0==str''cmp/; s/nullptr/NU''LL/' > k.c <<EOF || framework_failure_
 #define _GNU_SOURCE
 #include <stdio.h>
 #include <stdlib.h>
@@ -40,6 +40,35 @@ sed 's/streq/0==str''cmp/' > k.c <<EOF || framework_failure_
 #include <stdarg.h>
 #include <dlfcn.h>
 
+static FILE* (*fopen_func)(const char *, const char *);
+
+FILE* fopen(const char *path, const char *mode)
+{
+
+  /* get reference to original (libc provided) fopen */
+  if (!fopen_func)
+    {
+      fopen_func = (FILE*(*)(const char *, const char *))
+                   dlsym(RTLD_NEXT, "fopen");
+      if (!fopen_func)
+        {
+          fprintf (stderr, "Failed to find fopen()\n");
+          errno = ESRCH;
+          return nullptr;
+        }
+    }
+
+  /* Returning ENOENT here will get read_file_system_list()
+     to fall back to using getmntent() below.  */
+  if (streq (path, "/proc/self/mountinfo"))
+    {
+      errno = ENOENT;
+      return nullptr;
+    }
+
+  return fopen_func(path, mode);
+}
+
 int open(const char *path, int flags, ...)
 {
   static int (*open_func)(const char *, int, ...);
@@ -57,13 +86,6 @@ int open(const char *path, int flags, ...)
         }
     }
 
-  va_list ap;
-  va_start (ap, flags);
-  mode_t mode = (sizeof (mode_t) < sizeof (int)
-                 ? va_arg (ap, int)
-                 : va_arg (ap, mode_t));
-  va_end (ap);
-
   /* Returning ENOENT here will get read_file_system_list()
      to fall back to using getmntent() below.  */
   if (streq (path, "/proc/self/mountinfo"))
@@ -71,8 +93,15 @@ int open(const char *path, int flags, ...)
       errno = ENOENT;
       return -1;
     }
-  else
-    return open_func(path, flags, mode);
+
+  va_list ap;
+  va_start (ap, flags);
+  mode_t mode = (sizeof (mode_t) < sizeof (int)
+                 ? va_arg (ap, int)
+                 : va_arg (ap, mode_t));
+  va_end (ap);
+
+  return open_func(path, flags, mode);
 }
 
 struct mntent *getmntent (FILE *fp)
@@ -81,12 +110,12 @@ struct mntent *getmntent (FILE *fp)
   static int done = 0;
   if (!done)
     {
-      fclose (fopen ("x", "w"));
+      fclose (fopen_func ("x", "w"));
       ++done;
     }
   /* Now simulate the failure. */
   errno = ENOENT;
-  return NULL;
+  return nullptr;
 }
 EOF
 
@@ -99,7 +128,7 @@ cleanup_() { unset LD_PRELOAD; }
 export LD_PRELOAD=$LD_PRELOAD:./k.so
 
 # Test if LD_PRELOAD works:
-df 2>/dev/null
+df
 test -f x || skip_ "internal test failure: maybe LD_PRELOAD doesn't work?"
 
 # These tests are supposed to succeed:
diff --git a/tests/df/skip-duplicates.sh b/tests/df/skip-duplicates.sh
index c839f4e19..8521a9964 100755
--- a/tests/df/skip-duplicates.sh
+++ b/tests/df/skip-duplicates.sh
@@ -38,8 +38,8 @@ grep '^#define HAVE_GETMNTENT 1' $CONFIG_HEADER > /dev/null \
       || skip_ "getmntent is not used on this system"
 
 # Simulate an mtab file to test various cases.
-# Replace gnulib streq as that is not available here.
-sed 's/streq/0==str''cmp/' > k.c <<EOF || framework_failure_
+# Replace gnulib streq and C23 nullptr as that are not available here.
+sed 's/streq/0==str''cmp/; s/nullptr/NU''LL/' > k.c <<EOF || framework_failure_
 #define _GNU_SOURCE
 #include <stdio.h>
 #include <stdlib.h>
@@ -50,6 +50,35 @@ sed 's/streq/0==str''cmp/' > k.c <<EOF || framework_failure_
 #include <stdarg.h>
 #include <dlfcn.h>
 
+static FILE* (*fopen_func)(const char *, const char *);
+
+FILE* fopen(const char *path, const char *mode)
+{
+
+  /* get reference to original (libc provided) fopen */
+  if (!fopen_func)
+    {
+      fopen_func = (FILE*(*)(const char *, const char *))
+                   dlsym(RTLD_NEXT, "fopen");
+      if (!fopen_func)
+        {
+          fprintf (stderr, "Failed to find fopen()\n");
+          errno = ESRCH;
+          return nullptr;
+        }
+    }
+
+  /* Returning ENOENT here will get read_file_system_list()
+     to fall back to using getmntent() below.  */
+  if (streq (path, "/proc/self/mountinfo"))
+    {
+      errno = ENOENT;
+      return nullptr;
+    }
+
+  return fopen_func(path, mode);
+}
+
 int open(const char *path, int flags, ...)
 {
   static int (*open_func)(const char *, int, ...);
@@ -67,13 +96,6 @@ int open(const char *path, int flags, ...)
         }
     }
 
-  va_list ap;
-  va_start (ap, flags);
-  mode_t mode = (sizeof (mode_t) < sizeof (int)
-                 ? va_arg (ap, int)
-                 : va_arg (ap, mode_t));
-  va_end (ap);
-
   /* Returning ENOENT here will get read_file_system_list()
      to fall back to using getmntent() below.  */
   if (streq (path, "/proc/self/mountinfo"))
@@ -81,8 +103,15 @@ int open(const char *path, int flags, ...)
       errno = ENOENT;
       return -1;
     }
-  else
-    return open_func(path, flags, mode);
+
+  va_list ap;
+  va_start (ap, flags);
+  mode_t mode = (sizeof (mode_t) < sizeof (int)
+                 ? va_arg (ap, int)
+                 : va_arg (ap, mode_t));
+  va_end (ap);
+
+  return open_func(path, flags, mode);
 }
 
 struct mntent *getmntent (FILE *fp)
@@ -94,7 +123,7 @@ struct mntent *getmntent (FILE *fp)
   /* Prove that LD_PRELOAD works. */
   if (!done)
     {
-      fclose (fopen ("x", "w"));
+      fclose (fopen_func ("x", "w"));
       ++done;
     }
 
-- 
2.51.1

Reply via email to