[PATCH] string: Add tests for unique strerror error strings

[Florian Weimer]

* Arjun Shankar:

> diff --git a/string/tst-strerror-strings.c b/string/tst-strerror-strings.c
> new file mode 100644
> index 0000000000..4b53d9d821
> --- /dev/null
> +++ b/string/tst-strerror-strings.c
> @@ -0,0 +1,76 @@

> +static int
> +compare_strings (const void *a, const void *b)
> +{
> +  const char *stra = * (const char **) a;
> +  const char *strb = * (const char **) b;
> +
> +  int ret = strcmp (stra, strb);
> +
> +  if (!ret)
> +    FAIL_EXIT1 ("Found duplicate error strings: \"%s\"\n", stra);
> +
> +  return ret;
> +}

Hmm.  Is this really sufficient to catch equal strings?  I guess.  If
the sorting function uses transitivity to avoid comparing two entries,
they cannot be equal.  Otherwise it has to call compare_strings at least
once for the pair to see if they are indeed different.

> +
> +      /* Negative as well as large positive errnums are unused.  160 allows
> +         us to define more errors without needing to update this test.  */
> +      int is_unknown_error
> +        = (strstr (err_str[i], "Unknown error ") == err_str[i]);
> +      TEST_VERIFY_EXIT ((i >= 0 && i < 160) || is_unknown_error);
> +    }

I think the range is different on Hurd.  So this test would have to be
Linux-specific.

> +  /* We check for and fail on duplicate strings in the comparator.  */
> +  qsort (string, NSTRINGS, sizeof (char *), compare_strings);
> +
> +  return 0;

I think you should call free here, and make things consistent in terms
of memory allocation.

> +#include <support/test-driver.c>
> diff --git a/string/tst-strerror_l-strings.c b/string/tst-strerror_l-strings.c
> new file mode 100644
> index 0000000000..186a4b09c7
> --- /dev/null
> +++ b/string/tst-strerror_l-strings.c
> @@ -0,0 +1,38 @@

> +static locale_t loc = (locale_t) 0;
> +
> +/* Wrap strerror_l to be plugged into the equivalent strerror test.  */
> +static char *
> +wrap_strerror_l (int errnum)
> +{
> +  if (loc == (locale_t) 0)
> +    loc = xnewlocale (LC_ALL_MASK, "C", (locale_t) 0);
> +
> +  return xstrdup (strerror_l (errnum, loc));
> +}
> +
> +#define TEST_STRERROR_VARIANT wrap_strerror_l
> +#include "tst-strerror-strings.c"

I'd suggest a comment that (locale_t) 0 is the error return value of
newlocale and thus different from any valid locale_t value.

> diff --git a/string/tst-strerror_r-strings.c b/string/tst-strerror_r-strings.c
> new file mode 100644
> index 0000000000..feaccfbefb
> --- /dev/null
> +++ b/string/tst-strerror_r-strings.c

> +/* Wrap strerror_r into a checked variant that can be plugged into the
> +   equivalent strerror test.  */
> +static char *
> +test_and_return_strerror_r (int errnum)
> +{
> +  char *buf, *ret;
> +
> +  buf = xmalloc (1024);
> +  ret = strerror_r (errnum, buf, 1024);
> +
> +  /* "Unknown error" strings are returned in user supplied buffer.  */
> +  if (strstr (ret, "Unknown error ") == ret)
> +    TEST_VERIFY_EXIT (ret == buf);
> +  else
> +    {
> +      TEST_VERIFY_EXIT (ret != buf);
> +      free (buf);
> +    }
> +
> +  return ret;
> +}

I would just use a static buffer and call xstrdup on the strerror_r
result.

Thanks,
Florian