[PATCH v2] btrfs-progs: simplify mkfs_main cleanup

[PATCH v2] btrfs-progs: simplify mkfs_main cleanup

[Mark Harmstone]

mkfs_main is a main-like function, meaning that return and exit are
equivalent. Deduplicate our cleanup code by moving the error label.

Signed-off-by: Mark Harmstone <maharmstone@fb.com>
---
 mkfs/main.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/mkfs/main.c b/mkfs/main.c
index a69aa24b..a721acde 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -1158,6 +1158,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 					error("unrecognized filesystem feature '%s'",
 							tmp);
 					free(orig);
+					ret = 1;
 					goto error;
 				}
 				free(orig);
@@ -1179,6 +1180,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 					error("unrecognized runtime feature '%s'",
 					      tmp);
 					free(orig);
+					ret = 1;
 					goto error;
 				}
 				free(orig);
@@ -1245,8 +1247,10 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 
 	if (!sectorsize)
 		sectorsize = (u32)SZ_4K;
-	if (btrfs_check_sectorsize(sectorsize))
+	if (btrfs_check_sectorsize(sectorsize)) {
+		ret = 1;
 		goto error;
+	}
 
 	if (!nodesize)
 		nodesize = max_t(u32, sectorsize, BTRFS_MKFS_DEFAULT_NODE_SIZE);
@@ -1261,10 +1265,12 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 
 	if (source_dir && device_count > 1) {
 		error("the option -r is limited to a single device");
+		ret = 1;
 		goto error;
 	}
 	if (shrink_rootdir && source_dir == NULL) {
 		error("the option --shrink must be used with --rootdir");
+		ret = 1;
 		goto error;
 	}
 
@@ -1273,11 +1279,13 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 
 		if (uuid_parse(fs_uuid, dummy_uuid) != 0) {
 			error("could not parse UUID: %s", fs_uuid);
+			ret = 1;
 			goto error;
 		}
 		/* We allow non-unique fsid for single device btrfs filesystem. */
 		if (device_count != 1 && !test_uuid_unique(fs_uuid)) {
 			error("non-unique UUID: %s", fs_uuid);
+			ret = 1;
 			goto error;
 		}
 	}
@@ -1287,12 +1295,14 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 
 		if (uuid_parse(dev_uuid, dummy_uuid) != 0) {
 			error("could not parse device UUID: %s", dev_uuid);
+			ret = 1;
 			goto error;
 		}
 		/* We allow non-unique device uuid for single device filesystem. */
 		if (device_count != 1 && !test_uuid_unique(dev_uuid)) {
 			error("the option --device-uuid %s can be used only for a single device filesystem",
 			      dev_uuid);
+			ret = 1;
 			goto error;
 		}
 	}
@@ -1356,6 +1366,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 			if (metadata_profile != data_profile) {
 				error(
 	"with mixed block groups data and metadata profiles must be the same");
+				ret = 1;
 				goto error;
 			}
 		}
@@ -1425,12 +1436,15 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 			warning("libblkid < 2.38 does not support zoned mode's superblock location, update recommended");
 	}
 
-	if (btrfs_check_nodesize(nodesize, sectorsize, &features))
+	if (btrfs_check_nodesize(nodesize, sectorsize, &features)) {
+		ret = 1;
 		goto error;
+	}
 
 	if (sectorsize < sizeof(struct btrfs_super_block)) {
 		error("sectorsize smaller than superblock: %u < %zu",
 				sectorsize, sizeof(struct btrfs_super_block));
+		ret = 1;
 		goto error;
 	}
 
@@ -1461,6 +1475,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 					 S_IROTH);
 		if (fd < 0) {
 			error("unable to open %s: %m", file);
+			ret = 1;
 			goto error;
 		}
 
@@ -1506,6 +1521,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 		error("size %llu is too small to make a usable filesystem", byte_count);
 		error("minimum size for a %sbtrfs filesystem is %llu",
 		      opt_zoned ? "zoned mode " : "", min_dev_size);
+		ret = 1;
 		goto error;
 	}
 
@@ -1559,6 +1575,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 		if (!zoned_profile_supported(metadata, rst) ||
 		    !zoned_profile_supported(data, rst)) {
 			error("zoned mode does not yet support the selected RAID profiles");
+			ret = 1;
 			goto error;
 		}
 	}
@@ -1568,6 +1585,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 
 	if (!t_prepare || !prepare_ctx) {
 		error_msg(ERROR_MSG_MEMORY, "thread for preparing devices");
+		ret = 1;
 		goto error;
 	}
 
@@ -1609,6 +1627,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 	if (byte_count && byte_count > dev_byte_count) {
 		error("%s is smaller than requested size, expected %llu, found %llu",
 		      file, byte_count, dev_byte_count);
+		ret = 1;
 		goto error;
 	}
 
@@ -1652,6 +1671,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 	fs_info = open_ctree_fs_info(&oca);
 	if (!fs_info) {
 		error("open ctree failed");
+		ret = 1;
 		goto error;
 	}
 
@@ -1675,6 +1695,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 	if (IS_ERR(trans)) {
 		errno = -PTR_ERR(trans);
 		error_msg(ERROR_MSG_START_TRANS, "%m");
+		ret = 1;
 		goto error;
 	}
 
@@ -1709,6 +1730,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 	if (IS_ERR(trans)) {
 		errno = -PTR_ERR(trans);
 		error_msg(ERROR_MSG_START_TRANS, "%m");
+		ret = 1;
 		goto error;
 	}
 
@@ -1728,6 +1750,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 		if (prepare_ctx[i].ret) {
 			errno = -prepare_ctx[i].ret;
 			error("unable to prepare device %s: %m", prepare_ctx[i].file);
+			ret = 1;
 			goto error;
 		}
 
@@ -1776,6 +1799,7 @@ raid_groups:
 	if (IS_ERR(trans)) {
 		errno = -PTR_ERR(trans);
 		error_msg(ERROR_MSG_START_TRANS, "%m");
+		ret = 1;
 		goto error;
 	}
 	/* COW all tree blocks to newly created chunks */
@@ -1915,6 +1939,8 @@ out:
 	}
 
 	btrfs_close_all_devices();
+
+error:
 	if (prepare_ctx) {
 		for (i = 0; i < device_count; i++)
 			close(prepare_ctx[i].fd);
@@ -1926,16 +1952,6 @@ out:
 
 	return !!ret;
 
-error:
-	if (prepare_ctx) {
-		for (i = 0; i < device_count; i++)
-			close(prepare_ctx[i].fd);
-	}
-	free(t_prepare);
-	free(prepare_ctx);
-	free(label);
-	free(source_dir);
-	exit(1);
 success:
 	exit(0);
 }
-- 
2.44.2

Re: [PATCH v2] btrfs-progs: simplify mkfs_main cleanup

[David Sterba]

On Mon, Jul 22, 2024 at 03:32:24PM +0100, Mark Harmstone wrote:
> mkfs_main is a main-like function, meaning that return and exit are
> equivalent. Deduplicate our cleanup code by moving the error label.
> 
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>

Added to devel, thanks. It adds more code but at least removes the exit
block duplication. The 'success' label can be further moved before the
'return !!ret' so we don't have two exit points.