> 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@xxxxxxxxxx>
> ---
> 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()?
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list