[Libguestfs] [PATCH v4 2/3] do_btrfs_subvolume_list: fix a bad return value

[Pino Toscano]

On Wednesday 17 June 2015 16:19:32 Chen Hanxiao wrote:
> don't return a value which is to be freed.
> 
> v4: use strndup
> v3: v3: fix test case failure
> Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> ---
>  daemon/btrfs.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> index 8d03caa..5361984 100644
> --- a/daemon/btrfs.c
> +++ b/daemon/btrfs.c
> @@ -409,7 +409,7 @@ umount (char *fs_buf, const mountable_t *fs)
>  guestfs_int_btrfssubvolume_list *
>  do_btrfs_subvolume_list (const mountable_t *fs)
>  {
> -  char **lines;
> +  CLEANUP_FREE_STRING_LIST char **lines = NULL;
>    size_t i = 0;
>    const size_t MAX_ARGS = 64;
>    const char *argv[MAX_ARGS];
> @@ -472,7 +472,7 @@ do_btrfs_subvolume_list (const mountable_t *fs)
>    ret = malloc (sizeof *ret);
>    if (!ret) {
>      reply_with_perror ("malloc");
> -    goto error;
> +    return NULL;
>    }
>  
>    ret->guestfs_int_btrfssubvolume_list_len = nr_subvolumes;
> @@ -480,7 +480,8 @@ do_btrfs_subvolume_list (const mountable_t *fs)
>      calloc (nr_subvolumes, sizeof (struct guestfs_int_btrfssubvolume));
>    if (ret->guestfs_int_btrfssubvolume_list_val == NULL) {
>      reply_with_perror ("malloc");
> -    goto error;
> +    free (ret);
> +    return NULL;
>    }

You don't need the change here, if later in the "error:" label you keep
the "if (ret)" condition.

>    const char *errptr;
> @@ -530,20 +531,27 @@ do_btrfs_subvolume_list (const mountable_t *fs)
>  
>      #undef XSTRTOU64
>  
> -    memmove (line, line + ovector[6], ovector[7] - ovector[6] + 1);
> -    this->btrfssubvolume_path = line;
> +    this->btrfssubvolume_path =
> +      strndup (line + ovector[6], ovector[7] - ovector[6]);
> +    if (this->btrfssubvolume_path == NULL)
> +      goto error;
>    }
>  
> -  free (lines);
>    pcre_free (re);
>  
>    return ret;
>  
>  error:
> -  free_stringslen (lines, nr_subvolumes);
> -  if (ret) free (ret->guestfs_int_btrfssubvolume_list_val);
> +  for (i = 0; i < nr_subvolumes; ++i) {
> +    struct guestfs_int_btrfssubvolume *this =
> +      &ret->guestfs_int_btrfssubvolume_list_val[i];
> +    free (this->btrfssubvolume_path);

No need to save "this", just
  free (ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_path);
should be enough.

Thanks,
-- 
Pino Toscano