Ben Walton wrote:
> Provide a module that tests the functionality of sethostname().
Thanks, I'm applying this as well.
> Signed-off-by: Ben Walton <bwal...@artsci.utoronto.ca>
> ---
> ChangeLog | 6 ++
> modules/sethostname-tests | 13 +++++
> tests/test-sethostname.c | 117
> +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 136 insertions(+), 0 deletions(-)
> create mode 100644 modules/sethostname-tests
> create mode 100644 tests/test-sethostname.c
>
> diff --git a/ChangeLog b/ChangeLog
> index 67f7f42..2e85f16 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,11 @@
> 2011-12-01 Ben Walton <bwal...@artsci.utoronto.ca>
>
> + * modules/sethostname-tests: New file. A test program
> + for the sethostname module.
> + * tests/test-sethostname.c: Likewise.
> +
A trailing blank here (in place of a blank line) disturbs the
git-merge-changelog
program.
> +#define TESTHOSTNAME "gnulib-hostname"
> +
> +/* mingw and MSVC 9 lack geteuid, so setup a value that will indicate
> + we don't have root privilege since we wouldn't know whether to
> + expect success or failure when setting a name anyway*/
> +#if !HAVE_GETEUID
> +# define geteuid() ((uid_t) -1)
> +#endif
Wow! You have thought at many things. You did it better than if I had
written this test.
Just two things:
- If a test fails, it's most conventional (in Gnulib) to print the cause
to stderr, not to stdout.
- When we have to skip the major part of the test, by convention it should
return 77 (so that the Automake generated Makefile rule prints "SKIP:"
instead of "PASS:") and - by Gnulib convention - also print an explanation
why the test was skipped. This helps detecting bugs.
> + if (rcs != 0)
> + {
> + if (rcs == -1 && errno == ENOSYS)
> + return 0;
Oops, that looks like a tab again.
> + rcs = sethostname ((const char *) longname, (HOST_NAME_MAX + 1));
> +
> + if (rcs != -1)
> + {
> + /* attempt to restore the original name. */
> + sethostname (origname, strlen (origname));
> + printf ("expected failure when setting very long hostname.\n");
Error messages should not state in the first place what the program
expected, but what events occurred or what data actually appeared,
optionally with a small hint to what the program expected.
I'm applying these tweaks:
2011-12-03 Bruno Haible <br...@clisp.org>
Tweak last commit.
* modules/sethostname-tests (Files): Sort by decreasing importance.
(configure.ac): Check for geteuid.
* tests/test-sethostname.c (main): Emit error messages to stderr. Skip
the test when there's nothing to test. Drop an unnecessary cast.
Improve an error message. Verify that the final sethostname() call
succeeds.
--- modules/sethostname-tests.orig Sat Dec 3 14:38:36 2011
+++ modules/sethostname-tests Sat Dec 3 14:32:34 2011
@@ -1,12 +1,13 @@
Files:
-tests/signature.h
tests/test-sethostname.c
+tests/signature.h
tests/macros.h
Depends-on:
sys_types
configure.ac:
+AC_CHECK_FUNCS_ONCE([geteuid])
Makefile.am:
TESTS += test-sethostname
--- tests/test-sethostname.c.orig Sat Dec 3 14:38:36 2011
+++ tests/test-sethostname.c Sat Dec 3 14:38:16 2011
@@ -55,13 +55,16 @@
consider things like CAP_SYS_ADMIN (linux) or PRIV_SYS_ADMIN
(solaris), etc. systems without a working geteuid (mingw, MSVC
9) will always skip this test. */
- if (geteuid() != 0)
- return 0;
+ if (geteuid () != 0)
+ {
+ fprintf (stderr, "Skipping test: insufficient permissions.\n");
+ return 77;
+ }
/* we want to ensure we can do a get/set/get check to ensure the
change is accepted. record the current name so it can be restored
later */
- ASSERT(gethostname (origname, sizeof (origname)) == 0);
+ ASSERT (gethostname (origname, sizeof (origname)) == 0);
/* try setting a valid hostname. if it fails -1/ENOSYS, we will
skip the test for long names as this is an indication we're using
@@ -71,10 +74,14 @@
if (rcs != 0)
{
if (rcs == -1 && errno == ENOSYS)
- return 0;
+ {
+ fprintf (stderr,
+ "Skipping test: sethostname is not really implemented.\n");
+ return 77;
+ }
else
{
- printf ("error setting valid hostname.\n");
+ fprintf (stderr, "error setting valid hostname.\n");
return 1;
}
}
@@ -87,7 +94,7 @@
properly changed. */
if (strcmp (newname, TESTHOSTNAME) != 0)
{
- printf ("set/get comparison failed.\n");
+ fprintf (stderr, "set/get comparison failed.\n");
return 1;
}
}
@@ -100,18 +107,18 @@
longname[i] = '\0';
- rcs = sethostname ((const char *) longname, (HOST_NAME_MAX + 1));
+ rcs = sethostname (longname, (HOST_NAME_MAX + 1));
if (rcs != -1)
{
/* attempt to restore the original name. */
- sethostname (origname, strlen (origname));
- printf ("expected failure when setting very long hostname.\n");
+ ASSERT (sethostname (origname, strlen (origname)) == 0);
+ fprintf (stderr, "setting a too long hostname succeeded.\n");
return 1;
}
/* restore the original name. */
- sethostname (origname, strlen (origname));
+ ASSERT (sethostname (origname, strlen (origname)) == 0);
return 0;
}
--
In memoriam Rudolf Slánský <http://en.wikipedia.org/wiki/Rudolf_Slánský>
