[libvirt] [PATCH 6/7 v6] virsh: Don't motify the const string

[Peter Krempa]

On 09/14/12 18:21, Osier Yang wrote:
> This improve helper vshStringToArray to accept const string as
> argument instead. To not convert the const string when using
> vshStringToArray, and thus avoid motifying it.
> ---
>   tools/virsh-domain.c |    7 +++----
>   tools/virsh-pool.c   |    3 ++-
>   tools/virsh.c        |   10 ++++++----
>   tools/virsh.h        |    2 +-
>   4 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index c6695b3..3ad02eb 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -2336,8 +2336,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>       /* list of volumes to remove along with this domain */
>       vshUndefineVolume *vlist = NULL;
>       int nvols = 0;
> -    const char *volumes_arg = NULL;
> -    char *volumes = NULL;
> +    const char *volumes = NULL;
>       char **volume_tokens = NULL;
>       char *volume_tok = NULL;
>       int nvolume_tokens = 0;
> @@ -2352,8 +2351,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>       int nvolumes = 0;
>       bool vol_not_found = false;
>
> -    ignore_value(vshCommandOptString(cmd, "storage", &volumes_arg));
> -    volumes = vshStrdup(ctl, volumes_arg);
> +    ignore_value(vshCommandOptString(cmd, "storage", &volumes));
>
>       if (managed_save) {
>           flags |= VIR_DOMAIN_UNDEFINE_MANAGED_SAVE;
> @@ -2606,6 +2604,7 @@ cleanup:
>       VIR_FREE(vlist);
>
>       VIR_FREE(volumes);
> +    VIR_FREE(*volume_tokens);

Doing this guarantees a segfault. In many cases the token array may be 
NULL and you dereference it. This would cause a huge regression:

$ tools/virsh undefine test
Domain test has been undefined
Segmentation fault

You need to do:

if (volume_tokens) {
     VIR_FREE(*volume_tokens);
     VIR_FREE(volume_tokens);
}

That's the reason I suggested a macro.


>       VIR_FREE(volume_tokens);
>       VIR_FREE(def);
>       VIR_FREE(vol_nodes);