On 08/17/2012 09:38 AM, Osier Yang wrote:
> tools/virsh.c: New helper function vshStringToArray.
> tools/virsh-domain.c: use the helper in cmdUndefine.
> ---
> tools/virsh-domain.c | 19 ++-----------------
> tools/virsh.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 47 insertions(+), 17 deletions(-)
Umm...
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 676c002..d746e1e 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -2393,23 +2393,8 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>
> /* tokenize the string from user and save it's parts into an array */
> if (volumes) {
> - /* count the delimiters */
> - volume_tok = volumes;
> - nvolume_tokens = 1; /* we need at least one member */
> - while (*volume_tok) {
> - if (*(volume_tok++) == ',')
> - nvolume_tokens++;
> - }
> -
> - volume_tokens = vshCalloc(ctl, nvolume_tokens, sizeof(char *));
> -
> - /* tokenize the input string */
> - nvolume_tokens = 0;
> - volume_tok = volumes;
> - do {
> - volume_tokens[nvolume_tokens] = strsep(&volume_tok, ",");
> - nvolume_tokens++;
> - } while (volume_tok);
> + if ((nvolume_tokens = vshStringToArray(volumes, &volume_tokens)) < 0)
You are attempting to call...
> +++ b/tools/virsh.c
> @@ -3204,6 +3204,51 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
> return true;
> }
>
> +/*
> + * Convert the strings separated by ',' into array. The caller
> + * must free the returned array after use.
> + *
> + * Returns the length of the filled array on success, or -1
> + * on error.
> + */
> +static int
> +vshStringToArray(char *str,
> + char ***array)
...a static function from another file, with no prototype. How did this
compile for you? Oh, EWWWW. Did we really split virsh.c by doing
#include "virsh-*.c"? Yuck. A thousand times yuck.
We _really_ need to add headers (virsh.h for common utility functions
available throughout the other virsh helpers, and then headers like
virsh-pool.h that declares _just_ the arrays needed for virsh.c to know
the command structure provided by that category). That also means
fixing tools/Makefile.am to compile all of the C files. I'm okay with
factoring this out into a helper function, but not until we first clean
up virsh to use proper modules, rather than compiling a single .c that
includes all other .c files.
Since I mentioned it, I'll see if I can help.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list