diff mbox series

[U-Boot,3/7] common: kconfig: Mark AVB_VERIFY as dependent on PARTITION_UUIDS

Message ID 20180814004309.15271-3-erosca@de.adit-jv.com
State Accepted
Commit 87c814d4c773b1e311b972315c0a5e45fd32362f
Delegated to: Tom Rini
Headers show
Series [U-Boot,1/7] libavb: Handle wrong hashtree_error_mode in avb_append_options() | expand

Commit Message

Eugeniu Rosca Aug. 14, 2018, 12:43 a.m. UTC
Avoid below compiler [1] errors, reproduced with configuration [2]:

common/avb_verify.c: In function ‘get_unique_guid_for_partition’:
common/avb_verify.c:692:31: error: ‘disk_partition_t {aka struct disk_partition}’ has no member named ‘uuid’
  uuid_size = sizeof(part->info.uuid);
                               ^
common/avb_verify.c:696:29: error: ‘disk_partition_t {aka struct disk_partition}’ has no member named ‘uuid’
  memcpy(guid_buf, part->info.uuid, uuid_size);
                             ^
  LD      drivers/built-in.o
make[2]: *** [scripts/Makefile.build:278: common/avb_verify.o] Error 1

[1] aarch64-linux-gnu-gcc (Linaro GCC 7.2-2017.11)
[2] r8a7795_ulcb_defconfig, plus:
    CONFIG_AVB_VERIFY=y
    CONFIG_PARTITION_UUIDS=y
    CONFIG_UDP_FUNCTION_FASTBOOT=y
    CONFIG_LIBAVB=y

Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
---
 common/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Igor Opaniuk Aug. 16, 2018, 8:38 a.m. UTC | #1
Hi Eugeniu,

Why not keep all dependencies on the same line in this case? Simply:
depends LIBAVB && FASTBOOT && PARTITION_UUIDS

Regards,
Igor

On 14 August 2018 at 03:43, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
> Avoid below compiler [1] errors, reproduced with configuration [2]:
>
> common/avb_verify.c: In function ‘get_unique_guid_for_partition’:
> common/avb_verify.c:692:31: error: ‘disk_partition_t {aka struct disk_partition}’ has no member named ‘uuid’
>   uuid_size = sizeof(part->info.uuid);
>                                ^
> common/avb_verify.c:696:29: error: ‘disk_partition_t {aka struct disk_partition}’ has no member named ‘uuid’
>   memcpy(guid_buf, part->info.uuid, uuid_size);
>                              ^
>   LD      drivers/built-in.o
> make[2]: *** [scripts/Makefile.build:278: common/avb_verify.o] Error 1
>
> [1] aarch64-linux-gnu-gcc (Linaro GCC 7.2-2017.11)
> [2] r8a7795_ulcb_defconfig, plus:
>     CONFIG_AVB_VERIFY=y
>     CONFIG_PARTITION_UUIDS=y
>     CONFIG_UDP_FUNCTION_FASTBOOT=y
>     CONFIG_LIBAVB=y
>
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> ---
>  common/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 4d7215a36086..f48888a0e03b 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -640,6 +640,7 @@ config HASH
>  config AVB_VERIFY
>         bool "Build Android Verified Boot operations"
>         depends on LIBAVB && FASTBOOT
> +       depends on PARTITION_UUIDS
>         help
>           This option enables compilation of bootloader-dependent operations,
>           used by Android Verified Boot 2.0 library (libavb). Includes:
> --
> 2.18.0
>
Eugeniu Rosca Aug. 16, 2018, 6:25 p.m. UTC | #2
Hi Igor,

First, thanks for the reviews!

On Thu, Aug 16, 2018 at 11:38:18AM +0300, Igor Opaniuk wrote:
> Hi Eugeniu,
> 
> Why not keep all dependencies on the same line in this case? Simply:
> depends LIBAVB && FASTBOOT && PARTITION_UUIDS

I guess it's a matter of personal preference (but maybe not entirely).
Let's say one needs to replace "depends on PARTITION_UUIDS" with
"select PARTITION_UUIDS" due to e.g. a circular dependency detected
by Kconfig at some point. For me below two lines:

- depends on PARTITION_UUIDS
+ select PARTITION_UUIDS

look more readable and are easier to review than:

- depends LIBAVB && FASTBOOT && PARTITION_UUIDS
+ depends LIBAVB && FASTBOOT
+ select PARTITION_UUIDS

I still can update the patch. Just let me know.

> 
> Regards,
> Igor

Best regards,
Eugeniu.
Igor Opaniuk Aug. 17, 2018, 10:33 a.m. UTC | #3
Hi Eugeniu,

Makes sense, thanks for the explanation.

Reviewed-by: Igor Opaniuk <igor.opaniuk@linaro.org>

On 16 August 2018 at 21:25, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
> Hi Igor,
>
> First, thanks for the reviews!
>
> On Thu, Aug 16, 2018 at 11:38:18AM +0300, Igor Opaniuk wrote:
>> Hi Eugeniu,
>>
>> Why not keep all dependencies on the same line in this case? Simply:
>> depends LIBAVB && FASTBOOT && PARTITION_UUIDS
>
> I guess it's a matter of personal preference (but maybe not entirely).
> Let's say one needs to replace "depends on PARTITION_UUIDS" with
> "select PARTITION_UUIDS" due to e.g. a circular dependency detected
> by Kconfig at some point. For me below two lines:
>
> - depends on PARTITION_UUIDS
> + select PARTITION_UUIDS
>
> look more readable and are easier to review than:
>
> - depends LIBAVB && FASTBOOT && PARTITION_UUIDS
> + depends LIBAVB && FASTBOOT
> + select PARTITION_UUIDS
>
> I still can update the patch. Just let me know.
>
>>
>> Regards,
>> Igor
>
> Best regards,
> Eugeniu.
Tom Rini Aug. 24, 2018, 8:09 p.m. UTC | #4
On Tue, Aug 14, 2018 at 02:43:05AM +0200, Eugeniu Rosca wrote:

> Avoid below compiler [1] errors, reproduced with configuration [2]:
> 
> common/avb_verify.c: In function ‘get_unique_guid_for_partition’:
> common/avb_verify.c:692:31: error: ‘disk_partition_t {aka struct disk_partition}’ has no member named ‘uuid’
>   uuid_size = sizeof(part->info.uuid);
>                                ^
> common/avb_verify.c:696:29: error: ‘disk_partition_t {aka struct disk_partition}’ has no member named ‘uuid’
>   memcpy(guid_buf, part->info.uuid, uuid_size);
>                              ^
>   LD      drivers/built-in.o
> make[2]: *** [scripts/Makefile.build:278: common/avb_verify.o] Error 1
> 
> [1] aarch64-linux-gnu-gcc (Linaro GCC 7.2-2017.11)
> [2] r8a7795_ulcb_defconfig, plus:
>     CONFIG_AVB_VERIFY=y
>     CONFIG_PARTITION_UUIDS=y
>     CONFIG_UDP_FUNCTION_FASTBOOT=y
>     CONFIG_LIBAVB=y
> 
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@linaro.org>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/common/Kconfig b/common/Kconfig
index 4d7215a36086..f48888a0e03b 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -640,6 +640,7 @@  config HASH
 config AVB_VERIFY
 	bool "Build Android Verified Boot operations"
 	depends on LIBAVB && FASTBOOT
+	depends on PARTITION_UUIDS
 	help
 	  This option enables compilation of bootloader-dependent operations,
 	  used by Android Verified Boot 2.0 library (libavb). Includes: