Re: [libvirt] [PATCH] Introduce APIs for splitting/joining strings
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(a)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()?