[v2 1/6] test/os: add xstrtokenize test

[Peter Hutterer]

On Tue, Dec 17, 2013 at 11:35:54PM +0100, Daniel Martin wrote:
> Signed-off-by: Daniel Martin <consume.noise at gmail.com>
> ---
> First patch and highly unrelated, but the next one adds tests here too.
> And imo having tests doesn't hurt.

Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net> with a few nitpicks
below

> 
>  test/Makefile.am | 15 ++++++++++++++-
>  test/xstr.c      | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 1 deletion(-)
>  create mode 100644 test/xstr.c
> 
> diff --git a/test/Makefile.am b/test/Makefile.am
> index 2852bb3..097b3a4 100644
> --- a/test/Makefile.am
> +++ b/test/Makefile.am
> @@ -5,7 +5,19 @@ if XORG
>  # Tests that require at least some DDX functions in order to fully link
>  # For now, requires xf86 ddx, could be adjusted to use another
>  SUBDIRS += xi2
> -noinst_PROGRAMS += xkb input xtest misc fixes xfree86 hashtabletest os signal-logging touch
> +noinst_PROGRAMS += \
> +	fixes \
> +	hashtabletest \
> +	input \
> +	misc \
> +	os \
> +	signal-logging \
> +	touch \
> +	xfree86 \
> +	xkb \
> +	xstr \
> +	xtest
> +
>  endif
>  check_LTLIBRARIES = libxservertest.la
>  
> @@ -38,6 +50,7 @@ touch_LDADD=$(TEST_LDADD)
>  signal_logging_LDADD=$(TEST_LDADD)
>  hashtabletest_LDADD=$(TEST_LDADD) $(top_srcdir)/Xext/hashtable.c
>  os_LDADD=$(TEST_LDADD)
> +xstr_LDADD=$(TEST_LDADD)
>  
>  libxservertest_la_LIBADD = $(XSERVER_LIBS)
>  if XORG
> diff --git a/test/xstr.c b/test/xstr.c
> new file mode 100644
> index 0000000..aba3e86
> --- /dev/null
> +++ b/test/xstr.c
> @@ -0,0 +1,36 @@
> +#ifdef HAVE_DIX_CONFIG_H
> +#include <dix-config.h>
> +#endif
> +
> +#include "misc.h"
> +
> +static void test_xstrtokenize(void)
> +{
> +    char tokenstr[] = "123|456|789";
> +    char **tokens;
> +
> +    tokens = (char **) xstrtokenize(tokenstr, "|");

you don't need the cast here

> +
> +    assert(tokens);
> +    assert(*tokens);
> +    assert(*(tokens + 1));
> +    assert(*(tokens + 2));
> +    assert(*(tokens + 3) == NULL);
> +
> +    assert(strcmp(*tokens, "123") == 0);
> +    assert(strcmp(*(tokens + 1), "456") == 0);
> +    assert(strcmp(*(tokens + 2), "789") == 0);

I'd use tokens[0], tokens[1], etc, the *(tokens + 1) seems a bit
convoluted.

> +
> +    free(*tokens);
> +    free(*(tokens + 1));
> +    free(*(tokens + 2));
> +    free(tokens);
> +}

testing the easy case is good, but testing the odd cases is usually
better. in this case:
- behaviour for a zero-length token
- behaviour for a zero-length string
- behaviour for a string consisting only of tokens ("||||")
- behaviour for a string mixed with strings and tokens ("a|b||c")

these are the most likely cases to cause issues lateron, the standard
use-case is usually well-tested anyway :)

Cheers,
   Peter
 


> +
> +int
> +main(int argc, char **argv)
> +{
> +    test_xstrtokenize();
> +
> +    return 0;
> +}
> -- 
> 1.8.5.1