Hi Raymond, On Thu, 18 Jul 2024 at 17:46, Raymond Mao <[email protected]> wrote: > > Hi Simon, > > On Fri, 5 Jul 2024 at 04:36, Simon Glass <[email protected]> wrote: >> >> Hi, >> >> On Wed, Jul 3, 2024, 09:56 Ilias Apalodimas <[email protected]> >> wrote: >> > >> > Hi Raymond >> > >> > On Tue, 2 Jul 2024 at 21:27, Raymond Mao <[email protected]> wrote: >> > > >> > > Integrate common/hash.c on the hash shim layer so that hash APIs >> > > from mbedtls can be leveraged by boot/image and efi_loader. >> > > >> > > Signed-off-by: Raymond Mao <[email protected]> >> > > --- >> > > Changes in v2 >> > > - Use the original head files instead of creating new ones. >> > > Changes in v3 >> > > - Add handle checkers for malloc. >> > > Changes in v4 >> > > - None. >> > > >> > > common/hash.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> > > 1 file changed, 143 insertions(+) >> > > >> > > diff --git a/common/hash.c b/common/hash.c >> > > index ac63803fed9..96caf074374 100644 >> > > --- a/common/hash.c >> > > +++ b/common/hash.c >> > > @@ -35,6 +35,141 @@ >> > > #include <u-boot/sha512.h> >> > > #include <u-boot/md5.h> >> > > >> > > +#if CONFIG_IS_ENABLED(MBEDTLS_LIB_CRYPTO) >> > > + >> > > +static int hash_init_sha1(struct hash_algo *algo, void **ctxp) >> > > +{ >> > > + int ret; >> > > + mbedtls_sha1_context *ctx = malloc(sizeof(mbedtls_sha1_context)); >> >> >> Why do we need allocation here? We should avoid it where possible. >> > The API "hash_init_sha1(struct hash_algo *algo, void **ctxp)" is passing a > pointer > address and expecting to get the context from the pointer, it is reasonable > to do the > allocation. > On top of that, this patch doesn't make changes on this API itself, but just > adapted > it to MbedTLS stacks, thus you can see the allocation is needed by the > original API > as well.
Oh dear., I see Now I am looking at the code. It is full of #ifdefs for different cases. The whole thing needs a bit of a rationalisation before adding another case. Regards, Simon
