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 |
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 >
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.
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.
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 --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:
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(+)