Libguestfs - Jun 2015 - [PATCH v5 1/2] do_btrfs_qgroup_show: fix a bad return value

We should not use temporary lines buffer as return value,
for lines buffer will be freed.

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
v5: modify according to Pino's comments
v4: take advantage of sscanf's '%m'.
v3: fix test case failure

 daemon/btrfs.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 2f875ae..715d7d2 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -1249,7 +1249,7 @@ do_btrfs_qgroup_show (const char *path)
   CLEANUP_FREE char *err = NULL;
   CLEANUP_FREE char *out = NULL;
   int r;
-  char **lines;
+  CLEANUP_FREE_STRING_LIST char **lines = NULL;
 
   path_buf = sysroot_path (path);
   if (path_buf == NULL) {
@@ -1275,17 +1275,19 @@ do_btrfs_qgroup_show (const char *path)
   if (!lines)
     return NULL;
 
-  /* line 0 and 1 are:
+  /* Output of `btrfs qgroup show' is like:
+   *
+   *  qgroupid         rfer         excl
+   *  --------         ----         ----
+   *  0/5        9249849344   9249849344
    *
-   * qgroupid rfer          excl
-   * -------- ----          ----
    */
   size_t nr_qgroups = count_strings (lines) - 2;
   guestfs_int_btrfsqgroup_list *ret = NULL;
   ret = malloc (sizeof *ret);
   if (!ret) {
     reply_with_perror ("malloc");
-    goto error;
+    return NULL;
   }
 
   ret->guestfs_int_btrfsqgroup_list_len = nr_qgroups;
@@ -1300,31 +1302,23 @@ do_btrfs_qgroup_show (const char *path)
     char *line = lines[i + 2];
     struct guestfs_int_btrfsqgroup *this        
&ret->guestfs_int_btrfsqgroup_list_val[i];
-    uint64_t dummy1, dummy2;
-    char *p;
 
-    if (sscanf (line, "%" SCNu64 "/%" SCNu64 " %"
SCNu64 " %" SCNu64,
-                &dummy1, &dummy2, &this->btrfsqgroup_rfer,
-                &this->btrfsqgroup_excl) != 4) {
+    if (sscanf (line, "%m[0-9/] %" SCNu64 " %" SCNu64,
+                &this->btrfsqgroup_id, &this->btrfsqgroup_rfer,
+                &this->btrfsqgroup_excl) != 3) {
       reply_with_error ("cannot parse output of qgroup show command:
%s", line);
       goto error;
     }
-    p = strchr(line, ' ');
-    if (!p) {
-      reply_with_error ("truncated line: %s", line);
-      goto error;
-    }
-    *p = '\0';
-    this->btrfsqgroup_id = line;
   }
 
-  free (lines);
   return ret;
 
 error:
-  free_stringslen (lines, nr_qgroups + 2);
-  if (ret)
+  if (ret->guestfs_int_btrfsqgroup_list_val) {
+    for (i = 0; i < nr_qgroups; ++i)
+      free (ret->guestfs_int_btrfsqgroup_list_val[i].btrfsqgroup_id);
     free (ret->guestfs_int_btrfsqgroup_list_val);
+  }
   free (ret);
 
   return NULL;
-- 
2.1.0

don't return a value which is to be freed.

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
v5: modify according to Pino's comments
v4: use strndup
v3: fix test case failure

 daemon/btrfs.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 715d7d2..302f0fc 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;
@@ -530,20 +530,26 @@ 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);
+  if (ret->guestfs_int_btrfssubvolume_list_val) {
+    for (i = 0; i < nr_subvolumes; ++i)
+      free
(ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_path);
+    free (ret->guestfs_int_btrfssubvolume_list_val);
+  }
   free (ret);
-  if (re) pcre_free (re);
+
+  if (re)
+    pcre_free (re);
 
   return NULL;
 }
-- 
2.1.0

Thanks - I will push both of these, with an extra test, hopefully
later today.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org