[libvirt] [PATCH] Introduce APIs for splitting/joining strings

[Eric Blake]

> This introduces a few new APIs for dealing with strings.
> One to split a char * into a char **, another to join a
> char ** into a char *, and finally one to free a char **

Do we also want to migrate virsh.c:vshStringToArray() to
this file, with its additional magic of supporting ',,'
as an escape sequence for literal comma in one of the
strings being split?

> 
> There is a simple test suite to validate the edge cases
> too. No more need to use the horrible strtok_r() API,
> or hand-written code for splitting strings.
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  src/Makefile.am          |   1 +
>  src/libvirt_private.syms |   6 ++
>  src/util/virstring.c     | 129 +++++++++++++++++++++++++++++++++++++
>  src/util/virstring.h     |  36 +++++++++++
>  tests/Makefile.am        |   6 ++
>  tests/virstringtest.c    | 164
>  +++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 342 insertions(+)
>  create mode 100644 src/util/virstring.c
>  create mode 100644 src/util/virstring.h
>  create mode 100644 tests/virstringtest.c

> +
> +char **virStringSplit(const char *string,
> +                      const char *delim,
> +                      size_t max_tokens)
> +{
> +    char **tokens = NULL;
> +    size_t ntokens = 0;
> +    size_t maxtokens = 0;
> +    const char *remainder = string;
> +    char *tmp;
> +    size_t i;
> +
> +    if (max_tokens < 1)

Effectively 'if (!max_tokens)'

> +
> +char *virStringJoin(const char **strings,
> +                    const char *delim)
> +{

Should this function have a third argument that says how many
elements are in strings (and leave it 0 if strings is
NULL-terminated)?  Otherwise, callers will have to ensure
that there is a trailing NULL element in strings, instead
of being able to specifically request the joining of an
exact amount of strings.

> +    size_t len = 0;
> +    size_t delimlen = strlen(delim);
> +    const char **tmp = strings;
> +    char *string;
> +    char *offset;
> +
> +    while (tmp && *tmp) {
> +        len += strlen(*tmp);
> +        len += delimlen;
> +        tmp++;
> +    }

Would it be any easier to write this function in terms of
virBuffer, instead of rolling it by hand?  Untested:

char *virStringJoin(const char **strings,
                    const char *delim)
{
    virBuffer buf = VIR_BUFFER_INITIALIZER;
    if (strings && *strings) {
        virBufferAdd(&buf, *strings, 0);
        while (*++strings) {
            virBufferAdd(&buf, delim, 0);
            virBufferAdd(*strings);
        }
    }
    return virBufferContentAndReset(&buf);
}

> +void virStringFreeList(char **strings)

Another function that might benefit from a length
argument, rather than forcing the user to pass in a
NULL-terminated list; in which case, it would then be
usable in the no_memory label of the split function,
rather than your current hand-rolled cleanup loop
(since that label is a case of not having a NULL
terminator).

> +
> +char **virStringSplit(const char *string,
> +                      const char *delim,
> +                      size_t max_tokens);

Worth marking ATTRIBUTE_NONNULL(2)?  It looks like you
intend to allow NULL for arg 1 though (in which case the
return is also NULL).

> +
> +char *virStringJoin(const char **strings,
> +                    const char *delim);

Worth marking ATTRIBUTE_NONNULL(2)?

> +++ b/tests/Makefile.am
> @@ -95,6 +95,7 @@ test_programs = virshtest sockettest \
>  	virauthconfigtest \
>  	virbitmaptest \
>  	virlockspacetest \
> +	virstringtest \

Does .gitignore need to be updated for this binary?


> +++ b/tests/virstringtest.c
> @@ -0,0 +1,164 @@
> +/*
> + * Copyright (C) 2011 Red Hat, Inc.

2012

> +static int
> +mymain(void)
> +{
> +    int ret = 0;
> +
> +    signal(SIGPIPE, SIG_IGN);

Isn't this already taken care of in the top-level main() before
calling into mymain()?