diff mbox series

[v3,1/2] lib/uuid.c: use unique name for PARTITION_SYSTEM_GUID

Message ID 20250416074839.1267396-2-jerome.forissier@linaro.org
State New
Headers show
Series ut: fix print_guid() and enable UNIT_TEST for qemu_arm64 | expand

Commit Message

Jerome Forissier April 16, 2025, 7:48 a.m. UTC 
The name defined for PARTITION_SYSTEM_GUID in list_guid[] depends on
configuration options. It is "system" if CONFIG_PARTITION_TYPE_GUID is
enabled or "System Partition" if CONFIG_CMD_EFIDEBUG or CONFIG_EFI are
enabled. In addition, the unit test in test/common/print.c is incorrect
because it expects only "system" (or a hex GUID).

Make things more consistent by using a clear and unique name: "EFI
System Partition" whatever the configuration, and update the unit test
accordingly.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---

Changes in v3:
- Use a single entry in list_guid for PARTITION_SYSTEM_GUID

Changes in v2:
- Remove useless braces in if expression
- Change partition name and make it the same for all configs. Update the
  commit subject.

 lib/uuid.c          | 9 ++++-----
 test/common/print.c | 8 ++++----
 2 files changed, 8 insertions(+), 9 deletions(-)

Comments

Heinrich Schuchardt April 16, 2025, 10:01 a.m. UTC | #1 
Am 16. April 2025 09:48:20 MESZ schrieb Jerome Forissier <jerome.forissier@linaro.org>:
>The name defined for PARTITION_SYSTEM_GUID in list_guid[] depends on
>configuration options. It is "system" if CONFIG_PARTITION_TYPE_GUID is
>enabled or "System Partition" if CONFIG_CMD_EFIDEBUG or CONFIG_EFI are
>enabled. In addition, the unit test in test/common/print.c is incorrect
>because it expects only "system" (or a hex GUID).
>
>Make things more consistent by using a clear and unique name: "EFI
>System Partition" whatever the configuration, and update the unit test
>accordingly.
>
>Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

LGTM

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>


>---
>
>Changes in v3:
>- Use a single entry in list_guid for PARTITION_SYSTEM_GUID
>
>Changes in v2:
>- Remove useless braces in if expression
>- Change partition name and make it the same for all configs. Update the
>  commit subject.
>
> lib/uuid.c          | 9 ++++-----
> test/common/print.c | 8 ++++----
> 2 files changed, 8 insertions(+), 9 deletions(-)
>
>diff --git a/lib/uuid.c b/lib/uuid.c
>index 75658778044..6abbcf27b1f 100644
>--- a/lib/uuid.c
>+++ b/lib/uuid.c
>@@ -67,8 +67,11 @@ static const struct {
> 	efi_guid_t guid;
> } list_guid[] = {
> #ifndef USE_HOSTCC
>+#if defined(CONFIG_PARTITION_TYPE_GUID) || defined(CONFIG_CMD_EFIDEBUG) || \
>+	defined(CONFIG_EFI)
>+	{"EFI System Partition", PARTITION_SYSTEM_GUID},
>+#endif
> #ifdef CONFIG_PARTITION_TYPE_GUID
>-	{"system",	PARTITION_SYSTEM_GUID},
> 	{"mbr",		LEGACY_MBR_PARTITION_GUID},
> 	{"msft",	PARTITION_MSFT_RESERVED_GUID},
> 	{"data",	PARTITION_BASIC_DATA_GUID},
>@@ -182,10 +185,6 @@ static const struct {
> 	{
> 		"TCG2",
> 		EFI_TCG2_PROTOCOL_GUID,
>-		},
>-	{
>-		"System Partition",
>-		PARTITION_SYSTEM_GUID
> 	},
> 	{
> 		"Firmware Management",
>diff --git a/test/common/print.c b/test/common/print.c
>index e3711b10809..c48efc2783f 100644
>--- a/test/common/print.c
>+++ b/test/common/print.c
>@@ -45,11 +45,11 @@ static int print_guid(struct unit_test_state *uts)
> 	sprintf(str, "%pUL", guid);
> 	ut_asserteq_str("04030201-0605-0807-090A-0B0C0D0E0F10", str);
> 	sprintf(str, "%pUs", guid_esp);
>-	if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID)) { /* brace needed */
>-		ut_asserteq_str("system", str);
>-	} else {
>+	if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID) ||
>+	    IS_ENABLED(CONFIG_CMD_EFIDEBUG) || IS_ENABLED(CONFIG_EFI))
>+		ut_asserteq_str("EFI System Partition", str);
>+	else
> 		ut_asserteq_str("c12a7328-f81f-11d2-ba4b-00a0c93ec93b", str);
>-	}
> 	ret = snprintf(str, 4, "%pUL", guid);
> 	ut_asserteq(0, str[3]);
> 	ut_asserteq(36, ret);
Ilias Apalodimas April 16, 2025, 11:02 a.m. UTC | #2 
On Wed, 16 Apr 2025 at 10:48, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> The name defined for PARTITION_SYSTEM_GUID in list_guid[] depends on
> configuration options. It is "system" if CONFIG_PARTITION_TYPE_GUID is
> enabled or "System Partition" if CONFIG_CMD_EFIDEBUG or CONFIG_EFI are
> enabled. In addition, the unit test in test/common/print.c is incorrect
> because it expects only "system" (or a hex GUID).
>
> Make things more consistent by using a clear and unique name: "EFI
> System Partition" whatever the configuration, and update the unit test
> accordingly.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>
> Changes in v3:
> - Use a single entry in list_guid for PARTITION_SYSTEM_GUID
>
> Changes in v2:
> - Remove useless braces in if expression
> - Change partition name and make it the same for all configs. Update the
>   commit subject.
>
>  lib/uuid.c          | 9 ++++-----
>  test/common/print.c | 8 ++++----
>  2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/lib/uuid.c b/lib/uuid.c
> index 75658778044..6abbcf27b1f 100644
> --- a/lib/uuid.c
> +++ b/lib/uuid.c
> @@ -67,8 +67,11 @@ static const struct {
>         efi_guid_t guid;
>  } list_guid[] = {
>  #ifndef USE_HOSTCC
> +#if defined(CONFIG_PARTITION_TYPE_GUID) || defined(CONFIG_CMD_EFIDEBUG) || \
> +       defined(CONFIG_EFI)
> +       {"EFI System Partition", PARTITION_SYSTEM_GUID},
> +#endif
>  #ifdef CONFIG_PARTITION_TYPE_GUID
> -       {"system",      PARTITION_SYSTEM_GUID},
>         {"mbr",         LEGACY_MBR_PARTITION_GUID},
>         {"msft",        PARTITION_MSFT_RESERVED_GUID},
>         {"data",        PARTITION_BASIC_DATA_GUID},
> @@ -182,10 +185,6 @@ static const struct {
>         {
>                 "TCG2",
>                 EFI_TCG2_PROTOCOL_GUID,
> -               },
> -       {
> -               "System Partition",
> -               PARTITION_SYSTEM_GUID
>         },
>         {
>                 "Firmware Management",
> diff --git a/test/common/print.c b/test/common/print.c
> index e3711b10809..c48efc2783f 100644
> --- a/test/common/print.c
> +++ b/test/common/print.c
> @@ -45,11 +45,11 @@ static int print_guid(struct unit_test_state *uts)
>         sprintf(str, "%pUL", guid);
>         ut_asserteq_str("04030201-0605-0807-090A-0B0C0D0E0F10", str);
>         sprintf(str, "%pUs", guid_esp);
> -       if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID)) { /* brace needed */
> -               ut_asserteq_str("system", str);
> -       } else {
> +       if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID) ||
> +           IS_ENABLED(CONFIG_CMD_EFIDEBUG) || IS_ENABLED(CONFIG_EFI))
> +               ut_asserteq_str("EFI System Partition", str);
> +       else
>                 ut_asserteq_str("c12a7328-f81f-11d2-ba4b-00a0c93ec93b", str);
> -       }
>         ret = snprintf(str, 4, "%pUL", guid);
>         ut_asserteq(0, str[3]);
>         ut_asserteq(36, ret);
> --
> 2.43.0
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
diff mbox series

Patch

diff --git a/lib/uuid.c b/lib/uuid.c
index 75658778044..6abbcf27b1f 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -67,8 +67,11 @@  static const struct {
 	efi_guid_t guid;
 } list_guid[] = {
 #ifndef USE_HOSTCC
+#if defined(CONFIG_PARTITION_TYPE_GUID) || defined(CONFIG_CMD_EFIDEBUG) || \
+	defined(CONFIG_EFI)
+	{"EFI System Partition", PARTITION_SYSTEM_GUID},
+#endif
 #ifdef CONFIG_PARTITION_TYPE_GUID
-	{"system",	PARTITION_SYSTEM_GUID},
 	{"mbr",		LEGACY_MBR_PARTITION_GUID},
 	{"msft",	PARTITION_MSFT_RESERVED_GUID},
 	{"data",	PARTITION_BASIC_DATA_GUID},
@@ -182,10 +185,6 @@  static const struct {
 	{
 		"TCG2",
 		EFI_TCG2_PROTOCOL_GUID,
-		},
-	{
-		"System Partition",
-		PARTITION_SYSTEM_GUID
 	},
 	{
 		"Firmware Management",
diff --git a/test/common/print.c b/test/common/print.c
index e3711b10809..c48efc2783f 100644
--- a/test/common/print.c
+++ b/test/common/print.c
@@ -45,11 +45,11 @@  static int print_guid(struct unit_test_state *uts)
 	sprintf(str, "%pUL", guid);
 	ut_asserteq_str("04030201-0605-0807-090A-0B0C0D0E0F10", str);
 	sprintf(str, "%pUs", guid_esp);
-	if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID)) { /* brace needed */
-		ut_asserteq_str("system", str);
-	} else {
+	if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID) ||
+	    IS_ENABLED(CONFIG_CMD_EFIDEBUG) || IS_ENABLED(CONFIG_EFI))
+		ut_asserteq_str("EFI System Partition", str);
+	else
 		ut_asserteq_str("c12a7328-f81f-11d2-ba4b-00a0c93ec93b", str);
-	}
 	ret = snprintf(str, 4, "%pUL", guid);
 	ut_asserteq(0, str[3]);
 	ut_asserteq(36, ret);