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 <chenhanx...@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 _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs