Merge patch series "'part name' subcommand and some robustification"

Rasmus Villemoes <[email protected]> says:

Implement a "part name" subcommand, mirroring the existing "part
number" subcommand.

In the discussion for v1 of that, it came up that there's a bit of
inconsistency in how much and what one can assume to be initialized in
'struct disk_partition' after a successful call of one of the
get_info* family of functions. Patch 1/2 tries to consolidate
that by making sure all ->get_info invocations go through a common
helper that at least always initializes the string members.

Quentin, I've taken the liberty of including your Acks, as the
incremental diff in patch 1 is quite minor, but do speak up if I
should not have done that.

Link: https://lore.kernel.org/r/[email protected]

5 files changed, 83 insertions, 29 deletions

diff --git a/cmd/gpt.c b/cmd/gpt.c
index e18e5036a06..84221881c39 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -724,7 +724,7 @@ static int gpt_enumerate(struct blk_desc *desc)
 			continue;
 
 		for (i = 1; i < part_drv->max_entries; i++) {
-			ret = part_drv->get_info(desc, i, &pinfo);
+			ret = part_driver_get_info(part_drv, desc, i, &pinfo);
 			if (ret)
 				continue;
 
@@ -820,7 +820,7 @@ static int gpt_setenv(struct blk_desc *desc, const char *name)
 		int i;
 
 		for (i = 1; i < part_drv->max_entries; i++) {
-			ret = part_drv->get_info(desc, i, &pinfo);
+			ret = part_driver_get_info(part_drv, desc, i, &pinfo);
 			if (ret)
 				continue;
 
diff --git a/cmd/part.c b/cmd/part.c
index db7bc5819c0..975a0a08a99 100644
--- a/cmd/part.c
+++ b/cmd/part.c
@@ -25,7 +25,8 @@
 enum cmd_part_info {
 	CMD_PART_INFO_START = 0,
 	CMD_PART_INFO_SIZE,
-	CMD_PART_INFO_NUMBER
+	CMD_PART_INFO_NUMBER,
+	CMD_PART_INFO_NAME,
 };
 
 static int do_part_uuid(int argc, char *const argv[])
@@ -154,6 +155,9 @@ static int do_part_info(int argc, char *const argv[], enum cmd_part_info param)
 	case CMD_PART_INFO_NUMBER:
 		snprintf(buf, sizeof(buf), "0x%x", part);
 		break;
+	case CMD_PART_INFO_NAME:
+		snprintf(buf, sizeof(buf), "%s", info.name);
+		break;
 	default:
 		printf("** Unknown cmd_part_info value: %d\n", param);
 		return 1;
@@ -182,6 +186,11 @@ static int do_part_number(int argc, char *const argv[])
 	return do_part_info(argc, argv, CMD_PART_INFO_NUMBER);
 }
 
+static int do_part_name(int argc, char *const argv[])
+{
+	return do_part_info(argc, argv, CMD_PART_INFO_NAME);
+}
+
 static int do_part_set(int argc, char *const argv[])
 {
 	const char *devname, *partstr, *typestr;
@@ -273,6 +282,8 @@ static int do_part(struct cmd_tbl *cmdtp, int flag, int argc,
 		return do_part_size(argc - 2, argv + 2);
 	else if (!strcmp(argv[1], "number"))
 		return do_part_number(argc - 2, argv + 2);
+	else if (!strcmp(argv[1], "name"))
+		return do_part_name(argc - 2, argv + 2);
 	else if (!strcmp(argv[1], "types"))
 		return do_part_types(argc - 2, argv + 2);
 	else if (!strcmp(argv[1], "set"))
@@ -305,6 +316,9 @@ U_BOOT_CMD(
 	"part number <interface> <dev> <part> <varname>\n"
 	"    - set environment variable to the partition number using the partition name\n"
 	"      part must be specified as partition name\n"
+	"part name <interface> <dev> <part> <varname>\n"
+	"    - set environment variable to the partition name using the partition number\n"
+	"      part must be specified as partition number\n"
 #ifdef CONFIG_PARTITION_TYPE_GUID
 	"part type <interface> <dev>:<part>\n"
 	"    - print partition type\n"
diff --git a/disk/part.c b/disk/part.c
index be2b45d5a29..49a0fca6b89 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -72,6 +72,28 @@ struct part_driver *part_driver_lookup_type(struct blk_desc *desc)
 	return NULL;
 }
 
+static void disk_partition_clr(struct disk_partition *info)
+{
+	/* The common case is no UUID support */
+	disk_partition_clr_uuid(info);
+	disk_partition_clr_type_guid(info);
+	info->name[0] = '\0';
+	info->type[0] = '\0';
+}
+
+int part_driver_get_info(struct part_driver *drv, struct blk_desc *desc, int part,
+			 struct disk_partition *info)
+{
+	if (!drv->get_info) {
+		log_debug("## Driver %s does not have the get_info() method\n",
+			  drv->name);
+		return -ENOSYS;
+	}
+
+	disk_partition_clr(info);
+	return drv->get_info(desc, part, info);
+}
+
 int part_get_type_by_name(const char *name)
 {
 	struct part_driver *drv =
@@ -322,12 +344,9 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
 			  struct disk_partition *info)
 {
 	struct part_driver *drv;
+	int ret = -ENOENT;
 
 	if (blk_enabled()) {
-		/* The common case is no UUID support */
-		disk_partition_clr_uuid(info);
-		disk_partition_clr_type_guid(info);
-
 		if (part_type == PART_TYPE_UNKNOWN) {
 			drv = part_driver_lookup_type(desc);
 		} else {
@@ -339,18 +358,16 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
 			      desc->part_type);
 			return -EPROTONOSUPPORT;
 		}
-		if (!drv->get_info) {
-			PRINTF("## Driver %s does not have the get_info() method\n",
-			       drv->name);
-			return -ENOSYS;
-		}
-		if (drv->get_info(desc, part, info) == 0) {
+
+		ret = part_driver_get_info(drv, desc, part, info);
+		if (ret && ret != -ENOSYS) {
+			ret = -ENOENT;
+		} else {
 			PRINTF("## Valid %s partition found ##\n", drv->name);
-			return 0;
 		}
 	}
 
-	return -ENOENT;
+	return ret;
 }
 
 int part_get_info(struct blk_desc *desc, int part,
@@ -657,15 +674,12 @@ int part_get_info_by_name(struct blk_desc *desc, const char *name,
 	if (!part_drv)
 		return -1;
 
-	if (!part_drv->get_info) {
-		log_debug("## Driver %s does not have the get_info() method\n",
-			  part_drv->name);
-		return -ENOSYS;
-	}
-
 	for (i = 1; i < part_drv->max_entries; i++) {
-		ret = part_drv->get_info(desc, i, info);
+		ret = part_driver_get_info(part_drv, desc, i, info);
 		if (ret != 0) {
+			/* -ENOSYS means no ->get_info method. */
+			if (ret == -ENOSYS)
+				return ret;
 			/*
 			 * Partition with this index can't be obtained, but
 			 * further partitions might be, so keep checking.
@@ -695,15 +709,12 @@ int part_get_info_by_uuid(struct blk_desc *desc, const char *uuid,
 	if (!part_drv)
 		return -1;
 
-	if (!part_drv->get_info) {
-		log_debug("## Driver %s does not have the get_info() method\n",
-			  part_drv->name);
-		return -ENOSYS;
-	}
-
 	for (i = 1; i < part_drv->max_entries; i++) {
-		ret = part_drv->get_info(desc, i, info);
+		ret = part_driver_get_info(part_drv, desc, i, info);
 		if (ret != 0) {
+			/* -ENOSYS means no ->get_info method. */
+			if (ret == -ENOSYS)
+				return ret;
 			/*
 			 * Partition with this index can't be obtained, but
 			 * further partitions might be, so keep checking.
diff --git a/doc/usage/cmd/part.rst b/doc/usage/cmd/part.rst
index 72f5d8b8de7..299f2ac15c5 100644
--- a/doc/usage/cmd/part.rst
+++ b/doc/usage/cmd/part.rst
@@ -16,6 +16,7 @@ Synopsis
     part start <interface> <dev> <part> <varname>
     part size <interface> <dev> <part> <varname>
     part number <interface> <dev> <part> <varname>
+    part name <interface> <dev> <part> <varname>
     part set <interface> <dev> <part> <type>
     part type <interface> <dev>:<part> [varname]
     part types
@@ -86,6 +87,18 @@ part must be specified as partition name.
     varname
         a variable to store the current partition number value into
 
+The 'part name' command sets an environment variable to the partition name using the partition number,
+part must be specified as partition number.
+
+    interface
+        interface for accessing the block device (mmc, sata, scsi, usb, ....)
+    dev
+        device number
+    part
+        partition number
+    varname
+        a variable to store the current partition name into
+
 The 'part set' command sets the type of a partition. This is useful when
 autodetection fails or does not do the correct thing:
 
diff --git a/include/part.h b/include/part.h
index 6caaa6526aa..d940f8b5d0e 100644
--- a/include/part.h
+++ b/include/part.h
@@ -509,6 +509,22 @@ struct part_driver {
 	int (*test)(struct blk_desc *desc);
 };
 
+/**
+ * part_driver_get_info() - Call the part_driver's get_info method
+ *
+ * On success, string members of info are guaranteed to be properly
+ * initialized, though they may be empty.
+ *
+ * @drv:	Partition driver
+ * @desc:	Block device descriptor
+ * @part:	Partition number to read
+ * @info:	Returned partition information
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int part_driver_get_info(struct part_driver *drv, struct blk_desc *desc, int part,
+			 struct disk_partition *info);
+
 /* Declare a new U-Boot partition 'driver' */
 #define U_BOOT_PART_TYPE(__name)					\
 	ll_entry_declare(struct part_driver, __name, part_driver)