[U-Boot] [PATCH V3 4/8] disk: get_device_and_partition() enhancements

19 Sep 2012

From: Stephen Warren swarren@nvidia.com
Rework get_device_and_partition to:
a) Make use of get_device().
b) Add parameter to indicate whether returning a whole device is
   acceptable, or whether a partition is mandatory.
c) Make error-checking of the user's device-/partition-specification
   more complete. In particular, if strtoul() doesn't convert all
   characters, it's an error rather than just ignored.
d) Require user to explicitly specify "N:auto" as the device/partition
   specification to get the automatic "search for a bootable partition"
   mode of operation; Rob's patch changed the behaviour of some syntaxes
   from defaulting to partition 1.
The resultant device/partition returned by the function will be as
follows, based on whether the disk has a partition table (ptable) or not,
and whether the calling command allows the whole device to be returned
or not.
(D and P are integers, P >= 1)
D
D:
  No ptable:
    !allow_whole_dev: error
    allow_whole_dev: device D
  ptable:
    device D partition 1
D:0
  !allow_whole_dev: error
  allow_whole_dev: device D
D:P
  No ptable: error
  ptable: device D partition P
D:auto
  No ptable:
    !allow_whole_dev: error
    allow_whole_dev: device D
  ptable:
    first partition in device D with bootable flag set.
    If none, first valid paratition in device D.
Note: In order to review this patch, it's probably easiest to simply
look at the file contents post-application, rather than reading the
patch itself.
Signed-off-by: Stephen Warren swarren@nvidia.com
---
v3: New patch.
---
 common/cmd_disk.c       |    2 +-
 common/cmd_ext4.c       |    2 +-
 common/cmd_ext_common.c |    4 +-
 common/cmd_fat.c        |    8 +-
 common/cmd_reiser.c     |    4 +-
 common/cmd_zfs.c        |    4 +-
 disk/part.c             |  151 ++++++++++++++++++++++++++++++++++-------------
 include/part.h          |    9 ++-
 8 files changed, 127 insertions(+), 57 deletions(-)

diff --git a/common/cmd_disk.c b/common/cmd_disk.c
index e6676b0..3bd1eba 100644
--- a/common/cmd_disk.c
+++ b/common/cmd_disk.c
@@ -31,7 +31,7 @@ int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
    bootstage_mark(BOOTSTAGE_ID_IDE_BOOT_DEVICE);
part = get_device_and_partition(intf, (argc == 3) ? argv[2] : NULL,
-					&dev_desc, &info);
+					&dev_desc, &info, 1);
    if (part < 0) {
    	bootstage_error(BOOTSTAGE_ID_IDE_TYPE);
    	return 1;
diff --git a/common/cmd_ext4.c b/common/cmd_ext4.c
index 48f9ba3..ca46561 100644
--- a/common/cmd_ext4.c
+++ b/common/cmd_ext4.c
@@ -87,7 +87,7 @@ int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int argc,
    if (argc < 6)
    	return cmd_usage(cmdtp);
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
    if (part < 0)
    	return 1;
diff --git a/common/cmd_ext_common.c b/common/cmd_ext_common.c
index 7d26944..1952f4d 100644
--- a/common/cmd_ext_common.c
+++ b/common/cmd_ext_common.c
@@ -108,7 +108,7 @@ int do_ext_load(cmd_tbl_t *cmdtp, int flag, int argc,
    	return 1;
    }
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
    if (part < 0)
    	return 1;
@@ -166,7 +166,7 @@ int do_ext_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
    if (argc < 2)
    	return cmd_usage(cmdtp);
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
    if (part < 0)
    	return 1;
diff --git a/common/cmd_fat.c b/common/cmd_fat.c
index 90412d6..01e02f5 100644
--- a/common/cmd_fat.c
+++ b/common/cmd_fat.c
@@ -49,7 +49,7 @@ int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
    	return 1;
    }
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
    if (part < 0)
    	return 1;
@@ -101,7 +101,7 @@ int do_fat_ls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
    	return 0;
    }
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
    if (part < 0)
    	return 1;
@@ -139,7 +139,7 @@ int do_fat_fsinfo (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
    	return 0;
    }
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
    if (part < 0)
    	return 1;
@@ -175,7 +175,7 @@ static int do_fat_fswrite(cmd_tbl_t *cmdtp, int flag,
    if (argc < 5)
    	return cmd_usage(cmdtp);
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
    if (part < 0)
    	return 1;
diff --git a/common/cmd_reiser.c b/common/cmd_reiser.c
index 2865014..e658618 100644
--- a/common/cmd_reiser.c
+++ b/common/cmd_reiser.c
@@ -57,7 +57,7 @@ int do_reiserls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
    if (argc < 3)
    	return CMD_RET_USAGE;
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
    if (part < 0)
    	return 1;
@@ -140,7 +140,7 @@ int do_reiserload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
    	return 1;
    }
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
    if (part < 0)
    	return 1;
diff --git a/common/cmd_zfs.c b/common/cmd_zfs.c
index 27f8856..d580f7b 100644
--- a/common/cmd_zfs.c
+++ b/common/cmd_zfs.c
@@ -94,7 +94,7 @@ static int do_zfs_load(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
    	return 1;
    }
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
    if (part < 0)
    	return 1;
@@ -160,7 +160,7 @@ static int do_zfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
    if (argc == 4)
    	filename = argv[3];
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
    if (part < 0)
    	return 1;
diff --git a/disk/part.c b/disk/part.c
index 9920d48..9916708 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -24,6 +24,7 @@
 #include <common.h>
 #include <command.h>
 #include <ide.h>
+#include <malloc.h>
 #include <part.h>
#undef	PART_DEBUG
@@ -457,58 +458,126 @@ int get_device(const char *ifname, const char *dev_str,
    return dev;
 }
+#define PART_UNSPECIFIED -2
+#define PART_AUTO -1
 #define MAX_SEARCH_PARTITIONS 16
-int get_device_and_partition(const char *ifname, const char *dev_str,
+int get_device_and_partition(const char *ifname, const char *dev_part_str,
    		     block_dev_desc_t **dev_desc,
-			     disk_partition_t *info)
+			     disk_partition_t *info, int allow_whole_dev)
 {
-	int ret;
-	char *ep;
+	int ret = -1;
+	const char *part_str;
+	char *dup_str = NULL;
+	const char *dev_str;
    int dev;
-	block_dev_desc_t *desc;
+	char *ep;
    int p;
-	int part = 0;
-	char *part_str;
+	int part;
    disk_partition_t tmpinfo;
-	if (dev_str)
-		dev = simple_strtoul(dev_str, &ep, 16);
+	/* If no dev_part_str, use bootdevice environment variable */
+	if (!dev_part_str)
+		dev_part_str = getenv("bootdevice");
+
+	/* If still no dev_part_str, it's an error */
+	if (!dev_part_str) {
+		printf("** No device specified **\n");
+		goto cleanup;
+	}
-	if (!dev_str || (dev_str == ep)) {
-		dev_str = getenv("bootdevice");
-		if (dev_str)
-			dev = simple_strtoul(dev_str, &ep, 16);
-		if (!dev_str || (dev_str == ep))
-			goto err;
+	/* Separate device and partition ID specification */
+	part_str = strchr(dev_part_str, ':');
+	if (part_str) {
+		dup_str = strdup(dev_part_str);
+		dup_str[part_str - dev_part_str] = 0;
+		dev_str = dup_str;
+		part_str++;
+	} else {
+		dev_str = dev_part_str;
    }
-	desc = get_dev(ifname, dev);
-	if (!desc || (desc->type == DEV_TYPE_UNKNOWN))
-		goto err;
+	/* Look up the device */
+	dev = get_device(ifname, dev_str, dev_desc);
+	if (dev < 0)
+		goto cleanup;
+
+	/* Convert partition ID string to number */
+	if (!part_str || !*part_str) {
+		part = PART_UNSPECIFIED;
+	} else if (!strcmp(part_str, "auto")) {
+		part = PART_AUTO;
+	} else {
+		/* Something specified -> use exactly that */
+		part = (int)simple_strtoul(part_str, &ep, 16);
+		/*
+		 * Less than whole string converted,
+		 * or request for whole device, but caller requires partition.
+		 */
+		if (*ep || (part == 0 && !allow_whole_dev)) {
+			printf("** Bad partition specification %s %s **\n",
+			    ifname, dev_part_str);
+			goto cleanup;
+		}
+	}
-	if (desc->part_type == PART_TYPE_UNKNOWN) {
-		/* disk doesn't use partition table */
-		if (!desc->lba) {
-			printf("**Bad disk size - %s %d:0 **\n", ifname, dev);
-			return -1;
+	/*
+	 * No partition table on device,
+	 * or user requested partition 0 (entire device).
+	 */
+	if (((*dev_desc)->part_type == PART_TYPE_UNKNOWN) ||
+	    (part == 0)) {
+		if (!(*dev_desc)->lba) {
+			printf("** Bad device size - %s %s **\n", ifname,
+			       dev_str);
+			goto cleanup;
    	}
+
+		/*
+		 * If user specified a partition ID other than 0,
+		 * or the calling command only accepts partitions,
+		 * it's an error.
+		 */
+		if ((part > 0) || (!allow_whole_dev)) {
+			printf("** No partition table - %s %s **\n", ifname,
+			       dev_str);
+			goto cleanup;
+		}
+
    	info->start = 0;
-		info->size = desc->lba;
-		info->blksz = desc->blksz;
+		info->size = (*dev_desc)->lba;
+		info->blksz = (*dev_desc)->blksz;
+		info->bootable = 0;
+#ifdef CONFIG_PARTITION_UUIDS
+		info->uuid[0] = 0;
+#endif
-		*dev_desc = desc;
-		return 0;
+		ret = 0;
+		goto cleanup;
    }
-	part_str = strchr(dev_str, ':');
-	if (part_str) {
-		part = (int)simple_strtoul(++part_str, NULL, 16);
-		ret = get_partition_info(desc, part, info);
+	/*
+	 * Now there's known to be a partition table,
+	 * not specifying a partition means to pick partition 1.
+	 */
+	if (part == PART_UNSPECIFIED)
+		part = 1;
+
+	/*
+	 * If user didn't specify a partition number, or did specify something
+	 * other than "auto", use that partition number directly.
+	 */
+	if (part != PART_AUTO) {
+		ret = get_partition_info(*dev_desc, part, info);
+		if (ret) {
+			printf("** Invalid partition %d **\n", part);
+			goto cleanup;
+		}
    } else {
    	/*
    	 * Find the first bootable partition.
    	 * If none are bootable, fall back to the first valid partition.
    	 */
+		part = 0;
    	for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
    		ret = get_partition_info(*dev_desc, p, info);
    		if (ret)
@@ -542,23 +611,23 @@ int get_device_and_partition(const char *ifname, const char *dev_str,
    		if (p == MAX_SEARCH_PARTITIONS + 1)
    			*info = tmpinfo;
    		ret = 0;
+		} else {
+			printf("** No valid partitions found **\n");
+			goto cleanup;
    	}
    }
-	if (ret) {
-		printf("** Invalid partition %d, use `dev[:part]' **\n", part);
-		return -1;
-	}
    if (strncmp((char *)info->type, BOOT_PART_TYPE, sizeof(info->type)) != 0) {
    	printf("** Invalid partition type "%.32s""
    		" (expect "" BOOT_PART_TYPE "")\n",
    		info->type);
-		return -1;
+		ret  = -1;
+		goto cleanup;
    }
-	*dev_desc = desc;
-	return part;
+	ret = part;
+	goto cleanup;
- err:
-	puts("** Invalid boot device, use `dev[:part]' **\n");
-	return -1;
+cleanup:
+	free(dup_str);
+	return ret;
 }
diff --git a/include/part.h b/include/part.h
index 144df4e..3f780a1 100644
--- a/include/part.h
+++ b/include/part.h
@@ -114,9 +114,9 @@ void  init_part (block_dev_desc_t *dev_desc);
 void dev_print(block_dev_desc_t *dev_desc);
 int get_device(const char *ifname, const char *dev_str,
           block_dev_desc_t **dev_desc);
-int get_device_and_partition(const char *ifname, const char *dev_str,
+int get_device_and_partition(const char *ifname, const char *dev_part_str,
    		     block_dev_desc_t **dev_desc,
-			     disk_partition_t *info);
+			     disk_partition_t *info, int allow_whole_dev);
 #else
 static inline block_dev_desc_t *get_dev(const char *ifname, int dev)
 { return NULL; }
@@ -137,9 +137,10 @@ static inline int get_device(const char *ifname, const char *dev_str,
           block_dev_desc_t **dev_desc)
 { return -1; }
 static inline int get_device_and_partition(const char *ifname,
-					   const char *dev_str,
+					   const char *dev_part_str,
    				   block_dev_desc_t **dev_desc,
-					   disk_partition_t *info)
+					   disk_partition_t *info,
+					   int allow_whole_dev)
 { *dev_desc = NULL; return -1; }
 #endif
-- 
1.7.0.4


    