[PATCH 06/18] image: Drop IMAGE_ENABLE_SHA1

Alex G
2021-05-19T17:44:18Z

On 5/19/21 11:36 AM, Simon Glass wrote:
> Hi Alexandru,
>
> On Mon, 17 May 2021 at 10:38, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>>
>> From: Simon Glass <sjg@chromium.org>
>>
>> We already have a host Kconfig for SHA1. Use CONFIG_IS_ENABLED(SHA1)
>> directly in the code shared with the host build, so we can drop the
>> unnecessary indirection.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>> ---
>> common/image-fit.c | 2 +-
>> include/image.h | 8 --------
>> 2 files changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/common/image-fit.c b/common/image-fit.c
>> index e614643fe3..24e92a8e92 100644
>> --- a/common/image-fit.c
>> +++ b/common/image-fit.c
>> @@ -1218,7 +1218,7 @@ int calculate_hash(const void *data, int data_len, const char *algo,
>> CHUNKSZ_CRC32);
>> *((uint32_t *)value) = cpu_to_uimage(*((uint32_t *)value));
>> *value_len = 4;
>> - } else if (IMAGE_ENABLE_SHA1 && strcmp(algo, "sha1") == 0) {
>> + } else if (CONFIG_IS_ENABLED(SHA1) && strcmp(algo, "sha1") == 0) {
>
> This can only work if the my host Kconfig patch comes first. Otherwise
> this code will just be skipped on the host.

I was scratching my head too as to why this works in practice, but not
in theory. There is a #define CONFIG_SHA1 in image.h.

Although not a perfect fix, we go from two ways to enable SHA1 ("#define
IMAGE_ENABLE_SHA1", and "#define CONFIG_SHA1"), to just one. That's why
I think this change is an improvement, and part of this series.

Alex

>> sha1_csum_wd((unsigned char *)data, data_len,
>> (unsigned char *)value, CHUNKSZ_SHA1);
>> *value_len = 20;
>> diff --git a/include/image.h b/include/image.h
>> index 887a3270bd..8c718adba0 100644
>> --- a/include/image.h
>> +++ b/include/image.h
>> @@ -68,13 +68,9 @@ struct fdt_region;
>> # ifdef CONFIG_SPL_MD5
>> # define IMAGE_ENABLE_MD5 1
>> # endif
>> -# ifdef CONFIG_SPL_FIT_SHA1
>> -# define IMAGE_ENABLE_SHA1 1
>> -# endif
>> # else
>> # define IMAGE_ENABLE_CRC32 1
>> # define IMAGE_ENABLE_MD5 1
>> -# define IMAGE_ENABLE_SHA1 1
>> # endif
>>
>> #ifndef IMAGE_ENABLE_CRC32
>> @@ -85,10 +81,6 @@ struct fdt_region;
>> #define IMAGE_ENABLE_MD5 0
>> #endif
>>
>> -#ifndef IMAGE_ENABLE_SHA1
>> -#define IMAGE_ENABLE_SHA1 0
>> -#endif
>> -
>> #if defined(CONFIG_FIT_SHA256) || \
>> defined(CONFIG_SPL_FIT_SHA256)
>> #define IMAGE_ENABLE_SHA256 1
>> --
>> 2.31.1
>>
>
> Regards,
> Simon
>