[libvirt] [PATCH 06/45] list: Add helper to convert strings separated by ', ' to array

[Eric Blake]

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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120817/a6553f41/attachment-0001.sig>