[PATCH 06/18] image: Drop IMAGE_ENABLE_SHA1

[Alex G]

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 at gmail.com> wrote:
>>
>> From: Simon Glass <sjg at 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 at chromium.org>
>> Reviewed-by: Alexandru Gagniuc <mr.nuke.me at gmail.com>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me at 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
>