Re: [libvirt] [PATCH 1/2] virsh: make ,, escape parsing common

So far, none of the existing callers of vshStringToArray expected the user to ever pass a literal comma; meanwhile, snapshot parsing had rolled its own array parser. Moving the comma escaping into the common function won't affect any existing callers, and will make this function reusable for adding memory handling to snapshot parsing. As a bonus, the testsuite was already testing snapshot parsing, so the fact that the test still passes means that we are now giving testsuite exposure to vshStringToArray. * tools/virsh-snapshot.c (vshParseSnapshotDiskspec): Move ,, parsing... * tools/virsh.c (vshStringToArray): ...into common function. Also, vshStrdup can't fail. --- tools/virsh-snapshot.c | 50 ++++++++++++++++++++++------------------------- tools/virsh.c | 53 ++++++++++++++++++++++++++++++-------------------- 2 files changed, 55 insertions(+), 48 deletions(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 818ef56..159f577 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -197,33 +197,26 @@ static int vshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr buf, const char *str) { int ret = -1; - char *name = NULL; - char *snapshot = NULL; - char *driver = NULL; - char *file = NULL; - char *spec = vshStrdup(ctl, str); - char *tmp = spec; - size_t len = strlen(str); - - if (*str == ',') + const char *name = NULL; + const char *snapshot = NULL; + const char *driver = NULL; + const char *file = NULL; + char **array = NULL; + int narray; + int i; + + narray = vshStringToArray(str, &array); + if (narray <= 0) goto cleanup; - name = tmp; - while ((tmp = strchr(tmp, ','))) { - if (tmp[1] == ',') { - /* Recognize ,, as an escape for a literal comma */ - memmove(&tmp[1], &tmp[2], len - (tmp - spec) - 2 + 1); - len--; - tmp++; - continue; - } - /* Terminate previous string, look for next recognized one */ - *tmp++ = '\0'; - if (!snapshot && STRPREFIX(tmp, "snapshot=")) - snapshot = tmp + strlen("snapshot="); - else if (!driver && STRPREFIX(tmp, "driver=")) - driver = tmp + strlen("driver="); - else if (!file && STRPREFIX(tmp, "file=")) - file = tmp + strlen("file="); + + name = array[0]; + for (i = 1; i < narray; i++) { + if (!snapshot && STRPREFIX(array[i], "snapshot=")) + snapshot = array[i] + strlen("snapshot="); + else if (!driver && STRPREFIX(array[i], "driver=")) + driver = array[i] + strlen("driver="); + else if (!file && STRPREFIX(array[i], "file=")) + file = array[i] + strlen("file="); else goto cleanup; } @@ -245,7 +238,10 @@ vshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr buf, const char *str) cleanup: if (ret < 0) vshError(ctl, _("unable to parse diskspec: %s"), str); - VIR_FREE(spec); + if (array) { + VIR_FREE(*array); + VIR_FREE(array); + } return ret; } diff --git a/tools/virsh.c b/tools/virsh.c index a5585e1..9598b75 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -179,35 +179,46 @@ vshStringToArray(const char *str, { char *str_copied = vshStrdup(NULL, str); char *str_tok = NULL; + char *tmp; unsigned int nstr_tokens = 0; char **arr = NULL; + size_t len = strlen(str_copied);

/* tokenize the string from user and save it's parts into an array */ - if (str_copied) { - nstr_tokens = 1; - - /* count the delimiters */ - str_tok = str_copied; - while (*str_tok) { - if (*str_tok == ',') - nstr_tokens++; + nstr_tokens = 1; + + /* count the delimiters, recognizing ,, as an escape for a + * literal comma */ + str_tok = str_copied; + while ((str_tok = strchr(str_tok, ','))) { + if (str_tok[1] == ',') str_tok++; - } + else + nstr_tokens++; + str_tok++; + } - if (VIR_ALLOC_N(arr, nstr_tokens) < 0) { - virReportOOMError(); - VIR_FREE(str_copied); - return -1; - } + if (VIR_ALLOC_N(arr, nstr_tokens) < 0) { + virReportOOMError(); + VIR_FREE(str_copied); + return -1; + } - /* tokenize the input string */ - nstr_tokens = 0; - str_tok = str_copied; - do { - arr[nstr_tokens] = strsep(&str_tok, ","); - nstr_tokens++; - } while (str_tok); + /* tokenize the input string */ + nstr_tokens = 0; + tmp = str_tok = str_copied; + while ((tmp = strchr(tmp, ','))) { + if (tmp[1] == ',') { + memmove(&tmp[1], &tmp[2], len - (tmp - str_copied) - 2 + 1); + len--; + tmp++;

+ continue; + } + *tmp++ = '\0'; + arr[nstr_tokens++] = str_tok; + str_tok = tmp; } + arr[nstr_tokens++] = str_tok; *array = arr; return nstr_tokens; -- 1.7.11.7