[PATCH 06/18] image: Drop IMAGE_ENABLE_SHA1

[Alex G.]

On 5/19/21 4:55 PM, Simon Glass wrote:
> Hi Alex,
> 
> On Wed, 19 May 2021 at 11:44, Alex G <mr.nuke.me at gmail.com> wrote:
>>
>>
>>
>> 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.
> 
> No, we really should not do that...everything needs to be in Kconfig.

I agree for target code. But, as a long term solution, let's look at how 
we can get hash algos in linker lists, like we're proposing to do for 
crytpo algos. Or I could just drop this change in v2.

Alex