search for: free_stringslen

Declare most of the stringsbuf as CLEANUP_FREE_STRINGSBUF, so they are freed completely on stack unwind: use take_stringsbuf() in return places to take away from the stringsbuf its content, and remove all the manual calls to free_stringslen (no more needed now). This requires to not use free_stringslen anymore on failure in the helper functions of stringsbuf, which now leave the content as-is (might be still useful even on error). This allows us to simplify the memory management of stringsbuf's, which are not properly fully freed...

...(dir); - free (rawdev); return NULL; } diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index 8b9acc7..af151bd 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -512,6 +512,7 @@ add_string_nodup (struct stringsbuf *sb, char *str) reply_with_perror ("realloc"); free_stringslen (sb->argv, sb->size); sb->argv = NULL; + free (str); return -1; } sb->argv = new_argv; -- 2.5.5

These two patches are probably not completely independent, but separating them is a lot of work. With *both* patches applied, all the tests and extra-tests pass. That's no guarantee however that there isn't a mistake, so I don't think this patch is a candidate for the 1.16 branch, until it's had a lot more testing in development. Rich.

...; + "path\\s(.*)", + 0, &errptr, &erroffset, NULL); + if (re == NULL) { + reply_with_error ("pcre_compile (%i): %s", erroffset, errptr); + free (ret->guestfs_int_btrfssubvolume_list_val); + free (ret); + free_stringslen (lines, nr_subvolumes); + return NULL; + } + for (i = 0; i < nr_subvolumes; ++i) { /* To avoid allocations, reuse the 'line' buffer to store the * path. Thus we don't need to free 'line', since it will be * freed by the calling (XDR) code. */...

...- - strncpy (devname, d->d_name, sizeof devname); - devname[sizeof devname - 1] = '\0'; - - snprintf (buf, sizeof buf, "/sys/block/%s", devname); + DIR *dir = opendir (devdir); + if (!dir) { + reply_with_perror ("opendir: %s", devdir); + free_stringslen (r, size); + return 0; + } - dir2 = opendir (buf); - if (!dir2) { - reply_with_perror ("opendir: %s", buf); - free_stringslen (r, size); - return NULL; - } - while ((d = readdir (dir2)) != NULL) { - if (strncmp (d->d_name, devname, strlen (devname)) == 0)...

...nts.argv[i], NULL); if (r == -1) { reply_with_error ("umount: %s: %s", mounts.argv[i], err); + if (verbose) { + /* Try running lsof to see what is holding the mountpoint open. */ + command (NULL, NULL, "lsof", mounts.argv[i], NULL); + } free_stringslen (mounts.argv, mounts.size); return -1; } -- 2.4.3

As Pino's comment, we should take advantage of macro CLEANUP_FREE_STRING_LIST v4: remove some redundant strdup v3: fix test case failure v2: properly initialize lines Chen Hanxiao (3): do_btrfs_qgroup_show: fix a bad return value do_btrfs_subvolume_list: fix a bad return value btrfs: use CLEANUP_FREE_STRING_LIST for list free daemon/btrfs.c | 70

...ANUP_FREE_STRING_LIST char **lines; size_t i = 0; const size_t MAX_ARGS = 64; const char *argv[MAX_ARGS]; @@ -534,13 +534,11 @@ do_btrfs_subvolume_list (const mountable_t *fs) this->btrfssubvolume_path = line; } - free (lines); pcre_free (re); return ret; error: - free_stringslen (lines, nr_subvolumes); if (ret) free (ret->guestfs_int_btrfssubvolume_list_val); free (ret); if (re) pcre_free (re); @@ -1254,7 +1252,7 @@ do_btrfs_qgroup_show (const char *path) CLEANUP_FREE char *err = NULL; CLEANUP_FREE char *out = NULL; int r; - char **lines; + CLEANUP_F...

...EE_STRING_LIST char **lines = NULL; size_t i = 0; const size_t MAX_ARGS = 64; const char *argv[MAX_ARGS]; @@ -534,13 +534,11 @@ do_btrfs_subvolume_list (const mountable_t *fs) this->btrfssubvolume_path = line; } - free (lines); pcre_free (re); return ret; error: - free_stringslen (lines, nr_subvolumes); if (ret) free (ret->guestfs_int_btrfssubvolume_list_val); free (ret); if (re) pcre_free (re); @@ -1253,8 +1251,8 @@ do_btrfs_qgroup_show (const char *path) CLEANUP_FREE char *path_buf = NULL; CLEANUP_FREE char *err = NULL; CLEANUP_FREE char *out = NULL;...

...ems caused by temporary failures, such as a udev + * operation still completing, allow a second chance for each umount. + */ + r = command (NULL, NULL, str_umount, mounts.argv[i], NULL); if (r == -1) { - reply_with_error ("umount: %s: %s", mounts.argv[i], err); - free_stringslen (mounts.argv, mounts.size); - return -1; + CLEANUP_FREE char *err = NULL; + + sleep (5); + r = command (NULL, &err, str_umount, mounts.argv[i], NULL); + if (r == -1) { + reply_with_error ("umount: %s: %s", mounts.argv[i], err); + free_stringslen...

...ose"); + ret = pclose (fp); + if (ret != 0) { + if (ret == -1) + reply_with_perror ("pclose"); + else { + if (WEXITSTATUS (ret) != 0) + ret = WEXITSTATUS (ret); + reply_with_error ("pclose: command failed with return code %d", ret); + } free_stringslen (filenames.argv, filenames.size); return NULL; } -- 2.1.0

...d: %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; --...

...ly_with_error ("umount: %s: %s", mounts.argv[i], err); > > + if (verbose) { > > + /* Try running lsof to see what is holding the mountpoint open. */ > > + command (NULL, NULL, "lsof", mounts.argv[i], NULL); > > + } > > free_stringslen (mounts.argv, mounts.size); > > return -1; > > } > > LGTM. Maybe worth being done also on single umount API? Unfortunately there's no actual output from 'lsof'. I'm not sure if this is because I'm running lsof in the wrong way or (more likely) it...

...e, ' '); > - 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) > - free (ret->guestfs_int_btrfsqgroup_list_val); > + for (i = 0; i < nr_qgroups; ++i) { > + struct guestfs_int_btrfsqgroup *this = > + &ret->guestfs_int_btrfsqgroup_list_val[i]; > + free (this->btrfsqgroup_id);...

...mon/guestfsd.c b/daemon/guestfsd.c index c912ee3..453dee1 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -587,6 +587,13 @@ end_stringsbuf (struct stringsbuf *sb) return add_string_nodup (sb, NULL); } +void +free_stringsbuf (struct stringsbuf *sb) +{ + if (sb->argv != NULL) + free_stringslen (sb->argv, sb->size); +} + size_t count_strings (char *const *argv) { -- 2.1.0

...gt; + 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_...

Mofify the function from a fixed delimiter to a variable. So, it can be used in more APIs later. Also modified the existing callers Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> --- daemon/btrfs.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/daemon/btrfs.c b/daemon/btrfs.c index 39392f7..caa28ca 100644 --- a/daemon/btrfs.c +++ b/daemon/btrfs.c @@

As Pino's comment, we should take advantage of macro CLEANUP_FREE_STRING_LIST v3: fix test case failure v2: properly initialize lines Chen Hanxiao (3): do_btrfs_qgroup_show: fix a bad return value do_btrfs_subvolume_list: fix a bad return value btrfs: use CLEANUP_FREE_STRING_LIST for list free daemon/btrfs.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)

Mofify the function from a fixed delimiter to a variable. So, it can be used in more APIs later. Also modified the existing callers Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> --- daemon/btrfs.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/daemon/btrfs.c b/daemon/btrfs.c index 39392f7..caa28ca 100644 --- a/daemon/btrfs.c +++ b/daemon/btrfs.c @@

...4 +1320,14 @@ do_btrfs_qgroup_show (const char *path) goto error; } *p = '\0'; - this->btrfsqgroup_id = line; + this->btrfsqgroup_id = strdup (line); + if (this->btrfsqgroup_id == NULL) + goto error; } - free (lines); return ret; error: - free_stringslen (lines, nr_qgroups + 2); if (ret) free (ret->guestfs_int_btrfsqgroup_list_val); free (ret); -- 2.1.0