Re: [U-Boot] [PATCH v2 1/4] disk: part: implement generic function part_get_info_by_name()

[Simon Glass]

On 9 September 2016 at 02:27, Petr Kulhavy  wrote:
> So far partition search by name has been supported only on the EFI partition
> table. This patch extends the search to all partition tables.
>
> Rename part_get_info_efi_by_name() to part_get_info_by_name(), move it from
> part_efi.c into part.c and make it a generic function which traverses all part
> drivers and searches all partitions (in the order given by the linked list).
>
> For this a new variable struct part_driver.max_entries is added, which limits
> the number of partitions searched. For EFI this was GPT_ENTRY_NUMBERS.
> Similarly the limit is defined for DOS, ISO, MAC and AMIGA partition tables.
>
> Signed-off-by: Petr Kulhavy 
> Reviewed-by: Tom Rini 
> ---
> v1: initial
> v2: no change
>
>  common/fb_mmc.c   |  4 ++--
>  disk/part.c   | 26 ++
>  disk/part_amiga.c |  1 +
>  disk/part_dos.c   |  1 +
>  disk/part_efi.c   | 20 +---
>  disk/part_iso.c   |  1 +
>  disk/part_mac.c   |  1 +
>  include/part.h| 32 
>  8 files changed, 53 insertions(+), 33 deletions(-)

Reviewed-by: Simon Glass 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 1/4] disk: part: implement generic function part_get_info_by_name()

[Steve Rae]

Hi Petr,

On Fri, Sep 9, 2016 at 1:27 AM, Petr Kulhavy  wrote:
> So far partition search by name has been supported only on the EFI partition
> table. This patch extends the search to all partition tables.
>
> Rename part_get_info_efi_by_name() to part_get_info_by_name(), move it from
> part_efi.c into part.c and make it a generic function which traverses all part
> drivers and searches all partitions (in the order given by the linked list).
>
> For this a new variable struct part_driver.max_entries is added, which limits
> the number of partitions searched. For EFI this was GPT_ENTRY_NUMBERS.
> Similarly the limit is defined for DOS, ISO, MAC and AMIGA partition tables.
>
> Signed-off-by: Petr Kulhavy 
> Reviewed-by: Tom Rini 
> ---
> v1: initial
> v2: no change
>
>  common/fb_mmc.c   |  4 ++--
>  disk/part.c   | 26 ++
>  disk/part_amiga.c |  1 +
>  disk/part_dos.c   |  1 +
>  disk/part_efi.c   | 20 +---
>  disk/part_iso.c   |  1 +
>  disk/part_mac.c   |  1 +
>  include/part.h| 32 
>  8 files changed, 53 insertions(+), 33 deletions(-)
>
> diff --git a/common/fb_mmc.c b/common/fb_mmc.c
> index 8d0524d..a0a4a83 100644
> --- a/common/fb_mmc.c
> +++ b/common/fb_mmc.c
> @@ -27,7 +27,7 @@ static int part_get_info_efi_by_name_or_alias(struct 
> blk_desc *dev_desc,
>  {
> int ret;
>
> -   ret = part_get_info_efi_by_name(dev_desc, name, info);
> +   ret = part_get_info_by_name(dev_desc, name, info);
> if (ret) {
> /* strlen("fastboot_partition_alias_") + 32(part_name) + 1 */
> char env_alias_name[25 + 32 + 1];
> @@ -38,7 +38,7 @@ static int part_get_info_efi_by_name_or_alias(struct 
> blk_desc *dev_desc,
> strncat(env_alias_name, name, 32);
> aliased_part_name = getenv(env_alias_name);
> if (aliased_part_name != NULL)
> -   ret = part_get_info_efi_by_name(dev_desc,
> +   ret = part_get_info_by_name(dev_desc,
> aliased_part_name, info);
> }
> return ret;
> diff --git a/disk/part.c b/disk/part.c
> index 6a1c02d..8317e80 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -615,3 +615,29 @@ cleanup:
> free(dup_str);
> return ret;
>  }
> +
> +int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
> +   disk_partition_t *info)
> +{
> +   struct part_driver *first_drv =
> +   ll_entry_start(struct part_driver, part_driver);
> +   const int n_drvs = ll_entry_count(struct part_driver, part_driver);
> +   struct part_driver *part_drv;
> +
> +   for (part_drv = first_drv; part_drv != first_drv + n_drvs; 
> part_drv++) {
> +   int ret;
> +   int i;
> +   for (i = 1; i < part_drv->max_entries; i++) {
> +   ret = part_drv->get_info(dev_desc, i, info);
> +   if (ret != 0) {
> +   /* no more entries in table */
> +   break;
> +   }
> +   if (strcmp(name, (const char *)info->name) == 0) {
> +   /* matched */
> +   return 0;
> +   }
> +   }
> +   }
> +   return -1;
> +}

I am a little concerned about the additional feature in this code...
This function previously only searched for the "name" in the device
specified by "FASTBOOT_FLASH_MMC_DEV",
now it seems to be searching through multiple entries (... the
"part_drv" loop ...)
Anyway, it seems to still work properly on my boards, so I guess it is
OK for now



> diff --git a/disk/part_amiga.c b/disk/part_amiga.c
> index d4316b8..25fe56c 100644
> --- a/disk/part_amiga.c
> +++ b/disk/part_amiga.c
> @@ -381,6 +381,7 @@ static void part_print_amiga(struct blk_desc *dev_desc)
>  U_BOOT_PART_TYPE(amiga) = {
> .name   = "AMIGA",
> .part_type  = PART_TYPE_AMIGA,
> +   .max_entries= AMIGA_ENTRY_NUMBERS,
> .get_info   = part_get_info_amiga,
> .print  = part_print_amiga,
> .test   = part_test_amiga,
> diff --git a/disk/part_dos.c b/disk/part_dos.c
> index 511917a..8226601 100644
> --- a/disk/part_dos.c
> +++ b/disk/part_dos.c
> @@ -300,6 +300,7 @@ int part_get_info_dos(struct blk_desc *dev_desc, int part,
>  U_BOOT_PART_TYPE(dos) = {
> .name   = "DOS",
> .part_type  = PART_TYPE_DOS,
> +   .max_entries= DOS_ENTRY_NUMBERS,
> .get_info   = part_get_info_ptr(part_get_info_dos),
> .print  = part_print_ptr(part_print_dos),
> .test   = part_test_dos,
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index 8d67c09..1924338 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ 

[U-Boot] [PATCH v2 1/4] disk: part: implement generic function part_get_info_by_name()

[Petr Kulhavy]

So far partition search by name has been supported only on the EFI partition
table. This patch extends the search to all partition tables.

Rename part_get_info_efi_by_name() to part_get_info_by_name(), move it from
part_efi.c into part.c and make it a generic function which traverses all part
drivers and searches all partitions (in the order given by the linked list).

For this a new variable struct part_driver.max_entries is added, which limits
the number of partitions searched. For EFI this was GPT_ENTRY_NUMBERS.
Similarly the limit is defined for DOS, ISO, MAC and AMIGA partition tables.

Signed-off-by: Petr Kulhavy 
Reviewed-by: Tom Rini 
---
v1: initial
v2: no change

 common/fb_mmc.c   |  4 ++--
 disk/part.c   | 26 ++
 disk/part_amiga.c |  1 +
 disk/part_dos.c   |  1 +
 disk/part_efi.c   | 20 +---
 disk/part_iso.c   |  1 +
 disk/part_mac.c   |  1 +
 include/part.h| 32 
 8 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/common/fb_mmc.c b/common/fb_mmc.c
index 8d0524d..a0a4a83 100644
--- a/common/fb_mmc.c
+++ b/common/fb_mmc.c
@@ -27,7 +27,7 @@ static int part_get_info_efi_by_name_or_alias(struct blk_desc 
*dev_desc,
 {
int ret;
 
-   ret = part_get_info_efi_by_name(dev_desc, name, info);
+   ret = part_get_info_by_name(dev_desc, name, info);
if (ret) {
/* strlen("fastboot_partition_alias_") + 32(part_name) + 1 */
char env_alias_name[25 + 32 + 1];
@@ -38,7 +38,7 @@ static int part_get_info_efi_by_name_or_alias(struct blk_desc 
*dev_desc,
strncat(env_alias_name, name, 32);
aliased_part_name = getenv(env_alias_name);
if (aliased_part_name != NULL)
-   ret = part_get_info_efi_by_name(dev_desc,
+   ret = part_get_info_by_name(dev_desc,
aliased_part_name, info);
}
return ret;
diff --git a/disk/part.c b/disk/part.c
index 6a1c02d..8317e80 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -615,3 +615,29 @@ cleanup:
free(dup_str);
return ret;
 }
+
+int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
+   disk_partition_t *info)
+{
+   struct part_driver *first_drv =
+   ll_entry_start(struct part_driver, part_driver);
+   const int n_drvs = ll_entry_count(struct part_driver, part_driver);
+   struct part_driver *part_drv;
+
+   for (part_drv = first_drv; part_drv != first_drv + n_drvs; part_drv++) {
+   int ret;
+   int i;
+   for (i = 1; i < part_drv->max_entries; i++) {
+   ret = part_drv->get_info(dev_desc, i, info);
+   if (ret != 0) {
+   /* no more entries in table */
+   break;
+   }
+   if (strcmp(name, (const char *)info->name) == 0) {
+   /* matched */
+   return 0;
+   }
+   }
+   }
+   return -1;
+}
diff --git a/disk/part_amiga.c b/disk/part_amiga.c
index d4316b8..25fe56c 100644
--- a/disk/part_amiga.c
+++ b/disk/part_amiga.c
@@ -381,6 +381,7 @@ static void part_print_amiga(struct blk_desc *dev_desc)
 U_BOOT_PART_TYPE(amiga) = {
.name   = "AMIGA",
.part_type  = PART_TYPE_AMIGA,
+   .max_entries= AMIGA_ENTRY_NUMBERS,
.get_info   = part_get_info_amiga,
.print  = part_print_amiga,
.test   = part_test_amiga,
diff --git a/disk/part_dos.c b/disk/part_dos.c
index 511917a..8226601 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -300,6 +300,7 @@ int part_get_info_dos(struct blk_desc *dev_desc, int part,
 U_BOOT_PART_TYPE(dos) = {
.name   = "DOS",
.part_type  = PART_TYPE_DOS,
+   .max_entries= DOS_ENTRY_NUMBERS,
.get_info   = part_get_info_ptr(part_get_info_dos),
.print  = part_print_ptr(part_print_dos),
.test   = part_test_dos,
diff --git a/disk/part_efi.c b/disk/part_efi.c
index 8d67c09..1924338 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -296,25 +296,6 @@ int part_get_info_efi(struct blk_desc *dev_desc, int part,
return 0;
 }
 
-int part_get_info_efi_by_name(struct blk_desc *dev_desc,
-   const char *name, disk_partition_t *info)
-{
-   int ret;
-   int i;
-   for (i = 1; i < GPT_ENTRY_NUMBERS; i++) {
-   ret = part_get_info_efi(dev_desc, i, info);
-   if (ret != 0) {
-   /* no more entries in table */
-   return -1;
-   }
-   if (strcmp(name, (const char *)info->name) == 0) {
-   /* matched */
-   return 0;
-   }
-