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 <br...@clisp.org>
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 <br...@clisp.org>, 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;
}
