bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: getlogin tests


From: Bruno Haible
Subject: Re: getlogin tests
Date: Tue, 11 Jul 2017 00:54:41 +0200
User-agent: KMail/5.1.3 (Linux/4.4.0-83-generic; KDE/5.18.0; x86_64; ; )

Hi Paul,

> Looking at that test's source code, the test was clearly incorrect for 
> Unix-like systems, as it incorrectly assumed a 1-1 mapping between user 
> names and user IDs. I fixed that in Gnulib by installing the attached 
> patch.

Thanks. Indeed I had not thought at this situation when writing the test.

However, I dislike the use of #ifdef for code-sharing, when it can be avoided.
Rationale:
  - In the long run, it can lead to terribly intertwined modules, like [1],
    which took me an entire day to disentangle and make maintainable again.
  - Maintenance of code in this style is hard, because
    1. you have to realize that while the file is a .c file, it is being
       used by different modules,
    2. the cases are not symmetric: code for one case is labelled by
       #ifndef the guard for the other case.

I find 3 source files without #ifdefs *much* more maintainable than 2 source
files with #ifdefs.

[1] https://lists.gnu.org/archive/html/bug-gnulib/2017-05/msg00013.html


2017-07-10  Bruno Haible  <address@hidden>

        getlogin tests: Avoid #ifdefs when sharing code between modules.
        * modules/getlogin_r-tests (Files): Add tests/test-getlogin.h.
        * modules/getlogin-tests (Files): Likewise. Remove
        tests/test-getlogin_r.c.
        * tests/test-getlogin.h: Extracted from tests/test-getlogin_r.c.
        * tests/test-getlogin.c: Extracted from tests/test-getlogin_r.c.
        * tests/test-getlogin_r.c: Include test-getlogin.h. Omit code that tests
        getlogin().

diff --git a/modules/getlogin-tests b/modules/getlogin-tests
index d7d6aea..0ccd2eb 100644
--- a/modules/getlogin-tests
+++ b/modules/getlogin-tests
@@ -1,6 +1,6 @@
 Files:
 tests/test-getlogin.c
-tests/test-getlogin_r.c
+tests/test-getlogin.h
 tests/signature.h
 tests/macros.h
 
diff --git a/modules/getlogin_r-tests b/modules/getlogin_r-tests
index 845658f..190bc91 100644
--- a/modules/getlogin_r-tests
+++ b/modules/getlogin_r-tests
@@ -1,5 +1,6 @@
 Files:
 tests/test-getlogin_r.c
+tests/test-getlogin.h
 tests/signature.h
 tests/macros.h
 
diff --git a/tests/test-getlogin.c b/tests/test-getlogin.c
index 6a6d269..c4a6bc1 100644
--- a/tests/test-getlogin.c
+++ b/tests/test-getlogin.c
@@ -1,2 +1,38 @@
-#define TEST_GETLOGIN
-#include "test-getlogin_r.c"
+/* Test of getting user name.
+   Copyright (C) 2010-2017 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Bruno Haible and Paul Eggert.  */
+
+#include <config.h>
+
+#include <unistd.h>
+
+#include "signature.h"
+SIGNATURE_CHECK (getlogin, char *, (void));
+
+#include "test-getlogin.h"
+
+int
+main (void)
+{
+  /* Test value.  */
+  char *buf = getlogin ();
+  int err = buf ? 0 : errno;
+  ASSERT (buf || err);
+  test_getlogin_result (buf, err);
+
+  return 0;
+}
diff --git a/tests/test-getlogin.h b/tests/test-getlogin.h
new file mode 100644
index 0000000..30d9f4a
--- /dev/null
+++ b/tests/test-getlogin.h
@@ -0,0 +1,92 @@
+/* Test of getting user name.
+   Copyright (C) 2010-2017 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Bruno Haible and Paul Eggert.  */
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#if !((defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__)
+# include <pwd.h>
+#endif
+
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#include "macros.h"
+
+static void
+test_getlogin_result (const char *buf, int err)
+{
+  if (err != 0)
+    {
+      if (err == ENOENT)
+        {
+          /* This can happen on GNU/Linux.  */
+          fprintf (stderr, "Skipping test: no entry in utmp file.\n");
+          exit (77);
+        }
+
+      /* It fails when stdin is not connected to a tty.  */
+      ASSERT (err == ENOTTY
+              || err == EINVAL /* seen on Linux/SPARC */
+              || err == ENXIO
+             );
+#if !defined __hpux /* On HP-UX 11.11 it fails anyway.  */
+      ASSERT (! isatty (0));
+#endif
+      fprintf (stderr, "Skipping test: stdin is not a tty.\n");
+      exit (77);
+    }
+
+  /* Compare against the value from the environment.  */
+#if !((defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__)
+  /* Unix platform */
+  {
+    struct stat stat_buf;
+    struct passwd *pwd;
+
+    if (!isatty (STDIN_FILENO))
+      {
+         fprintf (stderr, "Skipping test: stdin is not a tty.\n");
+         exit (77);
+      }
+
+    ASSERT (fstat (STDIN_FILENO, &stat_buf) == 0);
+
+    pwd = getpwnam (buf);
+    if (! pwd)
+      {
+        fprintf (stderr, "Skipping test: %s: no such user\n", buf);
+        exit (77);
+      }
+
+    ASSERT (pwd->pw_uid == stat_buf.st_uid);
+  }
+#endif
+#if (defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__
+  /* Native Windows platform.
+     Note: This test would fail on Cygwin in an ssh session, because sshd
+     sets USERNAME=SYSTEM.  */
+  {
+    const char *name = getenv ("USERNAME");
+    if (name != NULL && name[0] != '\0')
+      ASSERT (strcmp (buf, name) == 0);
+  }
+#endif
+}
diff --git a/tests/test-getlogin_r.c b/tests/test-getlogin_r.c
index 0a7e105..81530f2 100644
--- a/tests/test-getlogin_r.c
+++ b/tests/test-getlogin_r.c
@@ -14,105 +14,26 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-/* Written by Bruno Haible <address@hidden>, 2010.  */
+/* Written by Bruno Haible and Paul Eggert.  */
 
 #include <config.h>
 
 #include <unistd.h>
 
 #include "signature.h"
-#ifdef TEST_GETLOGIN
-SIGNATURE_CHECK (getlogin, char *, (void));
-#else
 SIGNATURE_CHECK (getlogin_r, int, (char *, size_t));
-#endif
 
-#include <errno.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-#if !((defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__)
-# include <pwd.h>
-#endif
-
-#include <sys/stat.h>
-#include <sys/types.h>
-
-#include "macros.h"
+#include "test-getlogin.h"
 
 int
 main (void)
 {
   /* Test value.  */
-#ifdef TEST_GETLOGIN
-  char *buf = getlogin ();
-  int err = buf ? 0 : errno;
-  ASSERT (buf || err);
-#else
   /* Test with a large enough buffer.  */
   char buf[1024];
   int err = getlogin_r (buf, sizeof buf);
-#endif
-
-  if (err != 0)
-    {
-      if (err == ENOENT)
-        {
-          /* This can happen on GNU/Linux.  */
-          fprintf (stderr, "Skipping test: no entry in utmp file.\n");
-          return 77;
-        }
-
-      /* It fails when stdin is not connected to a tty.  */
-      ASSERT (err == ENOTTY
-              || err == EINVAL /* seen on Linux/SPARC */
-              || err == ENXIO
-             );
-#if !defined __hpux /* On HP-UX 11.11 it fails anyway.  */
-      ASSERT (! isatty (0));
-#endif
-      fprintf (stderr, "Skipping test: stdin is not a tty.\n");
-      return 77;
-    }
-
-  /* Compare against the value from the environment.  */
-#if !((defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__)
-  /* Unix platform */
-  {
-    struct stat stat_buf;
-    struct passwd *pwd;
-
-    if (!isatty (STDIN_FILENO))
-      {
-         fprintf (stderr, "Skipping test: stdin is not a tty.\n");
-         return 77;
-      }
-
-    ASSERT (fstat (STDIN_FILENO, &stat_buf) == 0);
-
-    pwd = getpwnam (buf);
-    if (! pwd)
-      {
-        fprintf (stderr, "Skipping test: %s: no such user\n", buf);
-        return 77;
-      }
-
-    ASSERT (pwd->pw_uid == stat_buf.st_uid);
-  }
-#endif
-#if (defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__
-  /* Native Windows platform.
-     Note: This test would fail on Cygwin in an ssh session, because sshd
-     sets USERNAME=SYSTEM.  */
-  {
-    const char *name = getenv ("USERNAME");
-    if (name != NULL && name[0] != '\0')
-      ASSERT (strcmp (buf, name) == 0);
-  }
-#endif
+  test_getlogin_result (buf, err);
 
-#ifndef TEST_GETLOGIN
   /* Test with a small buffer.  */
   {
     char smallbuf[1024];
@@ -136,7 +57,6 @@ main (void)
     ASSERT (getlogin_r (hugebuf, sizeof (hugebuf)) == 0);
     ASSERT (strcmp (hugebuf, buf) == 0);
   }
-#endif
 
   return 0;
 }




reply via email to

[Prev in Thread] Current Thread [Next in Thread]