[PATCH] Btrfs-progs: Add missing free() against fs_info->super_copy

[PATCH] Btrfs-progs: Add missing free() against fs_info->super_copy

[Filipe David Borba Manana]

There was a missing free() call against fs_info->super_copy
in several places:

1) close_ctree()

2) open_ctree_broken() on failure

3) __open_ctree_fd() on failure

Filipe David Borba Manana (1):
  Btrfs-progs: Add missing free() against fs_info->super_copy

 btrfs-find-root.c |    4 ++--
 disk-io.c         |    5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

-- 
1.7.9.5

[PATCH] Btrfs-progs: Add missing free() against fs_info->super_copy

[Filipe David Borba Manana]

There was a missing free() call against fs_info->super_copy
in several places:

1) close_ctree()

2) open_ctree_broken() on failure

3) __open_ctree_fd() on failure

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 btrfs-find-root.c |    4 ++--
 disk-io.c         |    5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/btrfs-find-root.c b/btrfs-find-root.c
index 810d835..5616e5c 100644
--- a/btrfs-find-root.c
+++ b/btrfs-find-root.c
@@ -94,7 +94,7 @@ static struct btrfs_root *open_ctree_broken(int fd, const char *device)
 	struct btrfs_root *chunk_root = malloc(sizeof(struct btrfs_root));
 	struct btrfs_root *dev_root = malloc(sizeof(struct btrfs_root));
 	struct btrfs_root *csum_root = malloc(sizeof(struct btrfs_root));
-	struct btrfs_fs_info *fs_info = malloc(sizeof(*fs_info));
+	struct btrfs_fs_info *fs_info = calloc(1, sizeof(*fs_info));
 	int ret;
 	struct btrfs_super_block *disk_super;
 	struct btrfs_fs_devices *fs_devices = NULL;
@@ -115,7 +115,6 @@ static struct btrfs_root *open_ctree_broken(int fd, const char *device)
 			goto out;
 	}
 
-	memset(fs_info, 0, sizeof(*fs_info));
 	fs_info->super_copy = calloc(1, BTRFS_SUPER_INFO_SIZE);
 	fs_info->tree_root = tree_root;
 	fs_info->extent_root = extent_root;
@@ -228,6 +227,7 @@ out:
 	free(chunk_root);
 	free(dev_root);
 	free(csum_root);
+	free(fs_info->super_copy);
 	free(fs_info);
 	return NULL;
 }
diff --git a/disk-io.c b/disk-io.c
index 21b410d..3b9b3ed 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -812,7 +812,7 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
 	struct btrfs_root *chunk_root = malloc(sizeof(struct btrfs_root));
 	struct btrfs_root *dev_root = malloc(sizeof(struct btrfs_root));
 	struct btrfs_root *csum_root = malloc(sizeof(struct btrfs_root));
-	struct btrfs_fs_info *fs_info = malloc(sizeof(*fs_info));
+	struct btrfs_fs_info *fs_info = calloc(1, sizeof(*fs_info));
 	int ret;
 	struct btrfs_super_block *disk_super;
 	struct btrfs_fs_devices *fs_devices = NULL;
@@ -840,7 +840,6 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
 			goto out;
 	}
 
-	memset(fs_info, 0, sizeof(*fs_info));
 	fs_info->super_copy = calloc(1, BTRFS_SUPER_INFO_SIZE);
 	fs_info->tree_root = tree_root;
 	fs_info->extent_root = extent_root;
@@ -1039,6 +1038,7 @@ out:
 	free(chunk_root);
 	free(dev_root);
 	free(csum_root);
+	free(fs_info->super_copy);
 	free(fs_info);
 	return NULL;
 }
@@ -1347,6 +1347,7 @@ int close_ctree(struct btrfs_root *root)
 	free(fs_info->chunk_root);
 	free(fs_info->dev_root);
 	free(fs_info->csum_root);
+	free(fs_info->super_copy);
 	free(fs_info);
 
 	return 0;
-- 
1.7.9.5

[PATCH] Btrfs-progs: Add missing free() against fs_info->super_copy

[Filipe David Borba Manana]

There was a missing free() call against fs_info->super_copy
in several places:

1) close_ctree()

2) open_ctree_broken() on failure

3) __open_ctree_fd() on failure

Filipe David Borba Manana (1):
  Btrfs-progs: Add missing free() against fs_info->super_copy

 btrfs-find-root.c |    5 +++--
 disk-io.c         |    6 ++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

-- 
1.7.9.5

[PATCH] Btrfs-progs: Add missing free() against fs_info->super_copy

[Filipe David Borba Manana]

There was a missing free() call against fs_info->super_copy
in several places:

1) close_ctree()

2) open_ctree_broken() on failure

3) __open_ctree_fd() on failure

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 btrfs-find-root.c |    5 +++--
 disk-io.c         |    6 ++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/btrfs-find-root.c b/btrfs-find-root.c
index 810d835..b2690d1 100644
--- a/btrfs-find-root.c
+++ b/btrfs-find-root.c
@@ -94,7 +94,7 @@ static struct btrfs_root *open_ctree_broken(int fd, const char *device)
 	struct btrfs_root *chunk_root = malloc(sizeof(struct btrfs_root));
 	struct btrfs_root *dev_root = malloc(sizeof(struct btrfs_root));
 	struct btrfs_root *csum_root = malloc(sizeof(struct btrfs_root));
-	struct btrfs_fs_info *fs_info = malloc(sizeof(*fs_info));
+	struct btrfs_fs_info *fs_info = calloc(1, sizeof(*fs_info));
 	int ret;
 	struct btrfs_super_block *disk_super;
 	struct btrfs_fs_devices *fs_devices = NULL;
@@ -115,7 +115,6 @@ static struct btrfs_root *open_ctree_broken(int fd, const char *device)
 			goto out;
 	}
 
-	memset(fs_info, 0, sizeof(*fs_info));
 	fs_info->super_copy = calloc(1, BTRFS_SUPER_INFO_SIZE);
 	fs_info->tree_root = tree_root;
 	fs_info->extent_root = extent_root;
@@ -228,6 +227,8 @@ out:
 	free(chunk_root);
 	free(dev_root);
 	free(csum_root);
+	if (fs_info)
+		free(fs_info->super_copy);
 	free(fs_info);
 	return NULL;
 }
diff --git a/disk-io.c b/disk-io.c
index 21b410d..eea7453 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -812,7 +812,7 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
 	struct btrfs_root *chunk_root = malloc(sizeof(struct btrfs_root));
 	struct btrfs_root *dev_root = malloc(sizeof(struct btrfs_root));
 	struct btrfs_root *csum_root = malloc(sizeof(struct btrfs_root));
-	struct btrfs_fs_info *fs_info = malloc(sizeof(*fs_info));
+	struct btrfs_fs_info *fs_info = calloc(1, sizeof(*fs_info));
 	int ret;
 	struct btrfs_super_block *disk_super;
 	struct btrfs_fs_devices *fs_devices = NULL;
@@ -840,7 +840,6 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
 			goto out;
 	}
 
-	memset(fs_info, 0, sizeof(*fs_info));
 	fs_info->super_copy = calloc(1, BTRFS_SUPER_INFO_SIZE);
 	fs_info->tree_root = tree_root;
 	fs_info->extent_root = extent_root;
@@ -1039,6 +1038,8 @@ out:
 	free(chunk_root);
 	free(dev_root);
 	free(csum_root);
+	if (fs_info)
+		free(fs_info->super_copy);
 	free(fs_info);
 	return NULL;
 }
@@ -1347,6 +1348,7 @@ int close_ctree(struct btrfs_root *root)
 	free(fs_info->chunk_root);
 	free(fs_info->dev_root);
 	free(fs_info->csum_root);
+	free(fs_info->super_copy);
 	free(fs_info);
 
 	return 0;
-- 
1.7.9.5

Re: [PATCH] Btrfs-progs: Add missing free() against fs_info->super_copy

[David Sterba]

Hi,

please don't forget to mention what changed in a revised version

On Sat, May 25, 2013 at 03:16:39PM +0100, Filipe David Borba Manana wrote:
> @@ -228,6 +227,8 @@ out:
>  	free(chunk_root);
>  	free(dev_root);
>  	free(csum_root);
> +	if (fs_info)
> +		free(fs_info->super_copy);

free accepts NULL pointers and does nothing, so it's fine to do it as in
the first version.

>  	free(fs_info);
>  	return NULL;

otherwise ok. Thanks.

david

Re: [PATCH] Btrfs-progs: Add missing free() against fs_info->super_copy

[Filipe David Manana]

On Mon, May 27, 2013 at 1:12 PM, David Sterba <dsterba@suse.cz> wrote:
> Hi,
>
> please don't forget to mention what changed in a revised version

Yeah, sorry, it was my first usage of git send-email (as described in the wiki).

>
> On Sat, May 25, 2013 at 03:16:39PM +0100, Filipe David Borba Manana wrote:
>> @@ -228,6 +227,8 @@ out:
>>       free(chunk_root);
>>       free(dev_root);
>>       free(csum_root);
>> +     if (fs_info)
>> +             free(fs_info->super_copy);
>
> free accepts NULL pointers and does nothing, so it's fine to do it as in
> the first version.

Right. Just thought it was better to check if the calloc() for fs_info
succeeded before derreferencing it for free'ing its super_copy member
(the function's code doesn't check if any of the malloc() / calloc()
calls returned non-NULL, so it wouldn't be inconsistent with the
existing code)

>
>>       free(fs_info);
>>       return NULL;
>
> otherwise ok. Thanks.

Thanks for review and advise.
Do you need more action from my side?

>
> david



--
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

Re: [PATCH] Btrfs-progs: Add missing free() against fs_info->super_copy

[David Sterba]

On Mon, May 27, 2013 at 01:31:13PM +0100, Filipe David Manana wrote:
> Right. Just thought it was better to check if the calloc() for fs_info
> succeeded before derreferencing it for free'ing its super_copy member
> (the function's code doesn't check if any of the malloc() / calloc()
> calls returned non-NULL, so it wouldn't be inconsistent with the
> existing code)

Sorry, I've read it too quickly, the check has to be there of course.

> >>       free(fs_info);
> >>       return NULL;
> >
> > otherwise ok. Thanks.
> 
> Thanks for review and advise.
> Do you need more action from my side?

No.

david