Re: [PATCH 3/5] getlogin-tests: remove xalloc dependency

[Bruno Haible via Gnulib discussion list]

Paul Eggert wrote:
> (main): Use a static rather than heap storage.

Thanks for the simplification.

> -  putenv (xstrdup ("LOGNAME=ygvfibmslhkmvoetbrcegzwydorcke"));
> +  static char set_LOGNAME[] = "LOGNAME=ygvfibmslhkmvoetbrcegzwydorcke";
> +  putenv (set_LOGNAME);
>    buf = getlogin ();
>    ASSERT (!(buf != NULL
> -            && strcmp (buf, "ygvfibmslhkmvoetbrcegzwydorcke") == 0));
> +            && strcmp (buf, set_LOGNAME + sizeof "LOGNAME") == 0));

I'm not happy about the change in the last line, though:

  - It adds an assumption about how putenv() operates. Namely, that
    after putenv() has added the string to the environment, that string
    will stay unmodified. It's more future-proof to not make this assumption
    and instead go by the rule: The program loses the ownership on the
    string once it has been passed to putenv().

  - sizeof "LOGNAME" means the size of the literal string "LOGNAME", plus
    the size needed for the trailing NUL. But here we don't have a trailing
    NUL; we have a "=" string which *happens* to have the same length.
    I find this way of writing   strlen ("LOGNAME") + strlen ("=")
    confusing.

The patch below reverts on that part, and generalizes to the 'getlogin_r'
tests.


2025-04-18  Bruno Haible  <br...@clisp.org>

        getlogin_r tests: Remove xalloc dependency.
        * tests/test-getlogin.c (main): Don't assume the putenv argument is
        unmodified.
        * tests/test-getlogin_r.c: Do not include xalloc.h.
        (main): Use a static rather than heap storage.
        * modules/getlogin_r-tests (Depends-on): Remove xalloc.
        (test_getlogin_LDADD): Remove @LIBINTL@.

diff --git a/modules/getlogin_r-tests b/modules/getlogin_r-tests
index 3cddb65584..8fefa059bc 100644
--- a/modules/getlogin_r-tests
+++ b/modules/getlogin_r-tests
@@ -6,11 +6,10 @@ tests/macros.h
 
 Depends-on:
 bool
-xalloc
 
 configure.ac:
 
 Makefile.am:
 TESTS += test-getlogin_r
 check_PROGRAMS += test-getlogin_r
-test_getlogin_r_LDADD = $(LDADD) @LIBINTL@ $(GETLOGIN_LIB)
+test_getlogin_r_LDADD = $(LDADD) $(GETLOGIN_LIB)
diff --git a/tests/test-getlogin.c b/tests/test-getlogin.c
index 35ca3e0980..d2e006d5e1 100644
--- a/tests/test-getlogin.c
+++ b/tests/test-getlogin.c
@@ -43,11 +43,13 @@ main (void)
   test_getlogin_result (buf, err);
 
   /* Check that getlogin() does not merely return getenv ("LOGNAME").  */
-  static char set_LOGNAME[] = "LOGNAME=ygvfibmslhkmvoetbrcegzwydorcke";
-  putenv (set_LOGNAME);
-  buf = getlogin ();
-  ASSERT (!(buf != NULL
-            && strcmp (buf, set_LOGNAME + sizeof "LOGNAME") == 0));
+  {
+    static char set_LOGNAME[] = "LOGNAME=ygvfibmslhkmvoetbrcegzwydorcke";
+    putenv (set_LOGNAME);
+    buf = getlogin ();
+    ASSERT (!(buf != NULL
+              && strcmp (buf, "ygvfibmslhkmvoetbrcegzwydorcke") == 0));
+  }
 
   return test_exit_status;
 }
diff --git a/tests/test-getlogin_r.c b/tests/test-getlogin_r.c
index e27e23da23..e8a438bf10 100644
--- a/tests/test-getlogin_r.c
+++ b/tests/test-getlogin_r.c
@@ -26,8 +26,6 @@
 SIGNATURE_CHECK (getlogin_r, int, (char *, size_t));
 #endif
 
-#include "xalloc.h"
-
 #include "test-getlogin.h"
 
 int
@@ -72,9 +70,12 @@ main (void)
   }
 
   /* Check that getlogin_r() does not merely return getenv ("LOGNAME").  */
-  putenv (xstrdup ("LOGNAME=ygvfibmslhkmvoetbrcegzwydorcke"));
-  err = getlogin_r (buf, sizeof buf);
-  ASSERT (!(err == 0 && strcmp (buf, "ygvfibmslhkmvoetbrcegzwydorcke") == 0));
+  {
+    static char set_LOGNAME[] = "LOGNAME=ygvfibmslhkmvoetbrcegzwydorcke";
+    putenv (set_LOGNAME);
+    err = getlogin_r (buf, sizeof buf);
+    ASSERT (!(err == 0 && strcmp (buf, "ygvfibmslhkmvoetbrcegzwydorcke") == 
0));
+  }
 
   return test_exit_status;
 }