[PATCH 1/2] btrfs-progs: add necessary close(fd) in mkfs

[PATCH 1/2] btrfs-progs: add necessary close(fd) in mkfs

[Gu Jinxiang]

Add some close(fd) when error occures in mkfs.
And add close(fd) when end use it.

Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com>
---
 mkfs/main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mkfs/main.c b/mkfs/main.c
index 2b109a5..ec82565 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -1694,6 +1694,7 @@ int main(int argc, char **argv)
 					file,
 					(unsigned long long)block_count,
 					(unsigned long long)dev_block_count);
+			close(fd);
 			exit(1);
 		}
 	} else {
@@ -1711,6 +1712,7 @@ int main(int argc, char **argv)
 		ret = zero_output_file(fd, block_count);
 		if (ret) {
 			error("unable to zero the output file");
+			close(fd);
 			exit(1);
 		}
 		/* our "device" is the new image file */
@@ -1721,6 +1723,7 @@ int main(int argc, char **argv)
 	if (dev_block_count < BTRFS_MKFS_SYSTEM_GROUP_SIZE) {
 		error("device is too small to make filesystem, must be at least %llu",
 				(unsigned long long)BTRFS_MKFS_SYSTEM_GROUP_SIZE);
+		close(fd);
 		exit(1);
 	}
 
@@ -1740,6 +1743,7 @@ int main(int argc, char **argv)
 	ret = make_btrfs(fd, &mkfs_cfg);
 	if (ret) {
 		error("error during mkfs: %s", strerror(-ret));
+		close(fd);
 		exit(1);
 	}
 
@@ -1750,6 +1754,7 @@ int main(int argc, char **argv)
 		close(fd);
 		exit(1);
 	}
+	close(fd);
 	root = fs_info->fs_root;
 	fs_info->alloc_start = alloc_start;
 
@@ -1827,6 +1832,7 @@ int main(int argc, char **argv)
 					sectorsize, sectorsize, sectorsize);
 		if (ret) {
 			error("unable to add %s to filesystem: %d", file, ret);
+			close(fd);
 			goto out;
 		}
 		if (verbose >= 2) {
-- 
1.9.1

[PATCH 2/2] btrfs-progs: delete not-used parameter fd

[Gu Jinxiang]

Parameter fd is not used in function make_image and
traverse_directory of mkfs.
Delete it.

Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com>
---
 mkfs/main.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/mkfs/main.c b/mkfs/main.c
index ec82565..3e9c1b8 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -801,7 +801,7 @@ static char *make_path(const char *dir, const char *name)
 
 static int traverse_directory(struct btrfs_trans_handle *trans,
 			      struct btrfs_root *root, const char *dir_name,
-			      struct directory_name_entry *dir_head, int out_fd)
+			      struct directory_name_entry *dir_head)
 {
 	int ret = 0;
 
@@ -1029,8 +1029,7 @@ static int create_chunks(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-static int make_image(const char *source_dir, struct btrfs_root *root,
-		int out_fd)
+static int make_image(const char *source_dir, struct btrfs_root *root)
 {
 	int ret;
 	struct btrfs_trans_handle *trans;
@@ -1048,7 +1047,7 @@ static int make_image(const char *source_dir, struct btrfs_root *root,
 	INIT_LIST_HEAD(&dir_head.list);
 
 	trans = btrfs_start_transaction(root, 1);
-	ret = traverse_directory(trans, root, source_dir, &dir_head, out_fd);
+	ret = traverse_directory(trans, root, source_dir, &dir_head);
 	if (ret) {
 		error("unable to traverse directory %s: %d", source_dir, ret);
 		goto fail;
@@ -1882,7 +1881,7 @@ raid_groups:
 			goto out;
 		}
 
-		ret = make_image(source_dir, root, fd);
+		ret = make_image(source_dir, root);
 		if (ret) {
 			error("error wihle filling filesystem: %d", ret);
 			goto out;
-- 
1.9.1

Re: [PATCH 1/2] btrfs-progs: add necessary close(fd) in mkfs

[Nikolay Borisov]

On 18.08.2017 11:32, Gu Jinxiang wrote:
> Add some close(fd) when error occures in mkfs.
> And add close(fd) when end use it.

When a process exits all of its fds (and all other runtime resources
tracked by the kernel) are freed, so we don't leak fds if that's your
worry. Admittedly, it's good practice to manage the lifetime of fds
correctly in a program, so I don't have any strong preferences either ways.


> 
> Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com>
> ---
>  mkfs/main.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 2b109a5..ec82565 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -1694,6 +1694,7 @@ int main(int argc, char **argv)
>  					file,
>  					(unsigned long long)block_count,
>  					(unsigned long long)dev_block_count);
> +			close(fd);
>  			exit(1);
>  		}
>  	} else {
> @@ -1711,6 +1712,7 @@ int main(int argc, char **argv)
>  		ret = zero_output_file(fd, block_count);
>  		if (ret) {
>  			error("unable to zero the output file");
> +			close(fd);
>  			exit(1);
>  		}
>  		/* our "device" is the new image file */
> @@ -1721,6 +1723,7 @@ int main(int argc, char **argv)
>  	if (dev_block_count < BTRFS_MKFS_SYSTEM_GROUP_SIZE) {
>  		error("device is too small to make filesystem, must be at least %llu",
>  				(unsigned long long)BTRFS_MKFS_SYSTEM_GROUP_SIZE);
> +		close(fd);
>  		exit(1);
>  	}
>  
> @@ -1740,6 +1743,7 @@ int main(int argc, char **argv)
>  	ret = make_btrfs(fd, &mkfs_cfg);
>  	if (ret) {
>  		error("error during mkfs: %s", strerror(-ret));
> +		close(fd);
>  		exit(1);
>  	}
>  
> @@ -1750,6 +1754,7 @@ int main(int argc, char **argv)
>  		close(fd);
>  		exit(1);
>  	}
> +	close(fd);
>  	root = fs_info->fs_root;
>  	fs_info->alloc_start = alloc_start;
>  
> @@ -1827,6 +1832,7 @@ int main(int argc, char **argv)
>  					sectorsize, sectorsize, sectorsize);
>  		if (ret) {
>  			error("unable to add %s to filesystem: %d", file, ret);
> +			close(fd);
>  			goto out;
>  		}
>  		if (verbose >= 2) {
> 

Re: [PATCH 1/2] btrfs-progs: add necessary close(fd) in mkfs

[David Sterba]

On Fri, Aug 18, 2017 at 01:32:45AM -0700, Gu Jinxiang wrote:
> Add some close(fd) when error occures in mkfs.
> And add close(fd) when end use it.

Can you please rework it so all the in-place exists are gotos to a
common exit block that does the close(fd) cleanup? Thanks.

Re: [PATCH 2/2] btrfs-progs: delete not-used parameter fd

[David Sterba]

On Fri, Aug 18, 2017 at 01:32:46AM -0700, Gu Jinxiang wrote:
> Parameter fd is not used in function make_image and
> traverse_directory of mkfs.
> Delete it.
> 
> Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com>

Apparently the parameter has never been used. Applied, thanks.