search for: btrfssubvolume_path

...ror:" 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...

...ucceeded at malloc(3) but failed at calloc(3), we will goto error. At this time we've got a space with uninitialized data because of malloc(3), but no space for guestfs_int_btrfsqgroup_list_val. When processing in label error, we could not know: ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_path is a valid address. 1) One solution is use calloc to replace the first malloc. Then: if (ret-> guestfs_int_btrfssubvolume_list_val) for (...) It costs more codes. 2) use the current solution I think the process in this patch should be a choice. How do you think? Regards, - Chen >...

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

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(-)

...eply_with_perror ("malloc"); - goto error; + free (ret); + return NULL; } 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) fre...

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

...subvolume. We get it by simply mounting + * the whole device of this btrfs filesystem. + */ + if (this->btrfssubvolume_id == (uint64_t) default_volume) + continue; + guestfs_int_add_sprintf (g, sb, "btrfsvol:%s/%s", device, this->btrfssubvolume_path); -- 2.5.0

...EANUP_FREE_STRING_LIST char **lines = NULL; size_t i = 0; const size_t MAX_ARGS = 64; const char *argv[MAX_ARGS]; @@ -531,16 +531,16 @@ 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 = strdup (line); + 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...

...on/btrfs.mli b/daemon/btrfs.mli index 8ca91fb47..ce1c2b66f 100644 --- a/daemon/btrfs.mli +++ b/daemon/btrfs.mli @@ -16,11 +16,5 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. *) -type btrfssubvolume = { - btrfssubvolume_id : int64; - btrfssubvolume_top_level_id : int64; - btrfssubvolume_path : string; -} - -val btrfs_subvolume_list : Mountable.t -> btrfssubvolume list +val btrfs_subvolume_list : Mountable.t -> Structs.btrfssubvolume list val btrfs_subvolume_get_default : Mountable.t -> int64 diff --git a/daemon/inspect_fs_windows.ml b/daemon/inspect_fs_windows.ml index e9d056...

...on/btrfs.mli b/daemon/btrfs.mli index 8ca91fb47..ce1c2b66f 100644 --- a/daemon/btrfs.mli +++ b/daemon/btrfs.mli @@ -16,11 +16,5 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. *) -type btrfssubvolume = { - btrfssubvolume_id : int64; - btrfssubvolume_top_level_id : int64; - btrfssubvolume_path : string; -} - -val btrfs_subvolume_list : Mountable.t -> btrfssubvolume list +val btrfs_subvolume_list : Mountable.t -> Structs.btrfssubvolume list val btrfs_subvolume_get_default : Mountable.t -> int64 diff --git a/daemon/inspect_fs_windows.ml b/daemon/inspect_fs_windows.ml index e9d056...

...&this->btrfssubvolume_id, NULL) != LONGINT_OK) - goto unexpected_output; - if (XSTRTOU64 (line + ovector[4], NULL, 10, - &this->btrfssubvolume_top_level_id, NULL) != LONGINT_OK) - goto unexpected_output; - -#undef XSTRTOU64 - - this->btrfssubvolume_path = - strndup (line + ovector[6], ovector[7] - ovector[6]); - if (this->btrfssubvolume_path == NULL) - goto error; - } - - return ret; - - error: - if (ret->guestfs_int_btrfssubvolume_list_val) { - for (i = 0; i < nr_subvolumes; ++i) - free (ret->guestfs_int_btrfss...

...&this->btrfssubvolume_id, NULL) != LONGINT_OK) - goto unexpected_output; - if (XSTRTOU64 (line + ovector[4], NULL, 10, - &this->btrfssubvolume_top_level_id, NULL) != LONGINT_OK) - goto unexpected_output; - -#undef XSTRTOU64 - - this->btrfssubvolume_path = - strndup (line + ovector[6], ovector[7] - ovector[6]); - if (this->btrfssubvolume_path == NULL) - goto error; - } - - return ret; - - error: - if (ret->guestfs_int_btrfssubvolume_list_val) { - for (i = 0; i < nr_subvolumes; ++i) - free (ret->guestfs_int_btrfss...

...fssubvolume_top_level_id = + STRTOU64(line + ovector[2], NULL, 10); + if (errno == ERANGE) goto unexpected_output; - memmove (line, pos, strlen (pos) + 1); + #undef STRTOU64 + + memmove (line, line + ovector[4], ovector[5] + 1); ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_path = line; } + pcre_free(re); + return ret; } -- 1.8.1

...c3247ac..765dec6 100644 --- a/daemon/btrfs.c +++ b/daemon/btrfs.c @@ -473,7 +473,7 @@ do_btrfs_subvolume_list (const mountable_t *fs) #undef XSTRTOU64 - memmove (line, line + ovector[6], ovector[7] + 1); + memmove (line, line + ovector[6], ovector[7] - ovector[6] + 1); this->btrfssubvolume_path = line; } -- 1.8.3.1

...mon/test-btrfs.pl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/daemon/test-btrfs.pl b/tests/daemon/test-btrfs.pl index 815ab1d..1430231 100755 --- a/tests/daemon/test-btrfs.pl +++ b/tests/daemon/test-btrfs.pl @@ -63,13 +63,13 @@ EOF die unless $r[2]->{btrfssubvolume_path} eq "test3"; # Test btrfs_qgroup_show. - my $output = <<EOF; -qgroupid rfer excl --------- ---- ---- -0/5 4096 4096 + $output = <<EOF; +qgroupid rfer excl +-------- ---- ---- +0/5 4096 4096 EOF set_btrfs_output ($output); - my @r = $g->btr...

...this btrfs filesystem. > > + */ > > + if (this->btrfssubvolume_id == (uint64_t) default_volume) > > + continue; > > + > > guestfs_int_add_sprintf (g, sb, > > "btrfsvol:%s/%s", > > device, this->btrfssubvolume_path); > > ACK. > > Was there an RHBZ# for this? Can't seem to find it ... I least none that I am aware of, not even a bsc# as we have an pretty old version on openSUSE. I just got the problem with my testing of recent releases. -- Cedric

...ed at calloc(3), > we will goto error. > > At this time we've got a space with uninitialized data because of malloc(3), > but no space for guestfs_int_btrfsqgroup_list_val. > When processing in label error, we could not know: > ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_path > is a valid address. > > 1) One solution is use calloc to replace the first malloc. > Then: > if (ret-> guestfs_int_btrfssubvolume_list_val) > for (...) > > It costs more codes. > > 2) use the current solution > > I think the process in this patch...

...int_btrfssubvolume_list * do_btrfs_subvolume_list (const mountable_t *fs) { - char **lines; + CLEANUP_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...

...fssubvolume_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]; @@ -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...

...> + * the whole device of this btrfs filesystem. > + */ > + if (this->btrfssubvolume_id == (uint64_t) default_volume) > + continue; > + > guestfs_int_add_sprintf (g, sb, > "btrfsvol:%s/%s", > device, this->btrfssubvolume_path); ACK. Was there an RHBZ# for this? Can't seem to find it ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and bu...