Re: [U-Boot] [PATCH 8/9] [v3] hash: Add function to find hash_algo struct with progressive hash

29 Dec 2014

Hi Simon,
...
-----Original Message-----
From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass
Sent: Wednesday, December 24, 2014 6:20 AM
To: Gupta Ruchika-R66431
Cc: U-Boot Mailing List; Sun York-R58495
Subject: Re: [PATCH 8/9] [v3] hash: Add function to find hash_algo struct
with progressive hash
Hi Ruchika,
On 23 December 2014 at 04:32, Ruchika Gupta ruchika.gupta@freescale.com
wrote:
...
The hash_algo structure has some implementations in which progressive
hash API's are not defined. These are basically the hardware based
implementations of SHA. An API is added to find the algo which has
progressive hash API's defined. This can then be integrated with RSA
checksum library which uses Progressive Hash API's.
Signed-off-by: Ruchika Gupta ruchika.gupta@freescale.com
CC: Simon Glass sjg@chromium.org

Changes in v3 :
Corrected ifdef for SHA1
Changes in v2 :
Added commit message
common/hash.c  | 33 ++++++++++++++++++++++++---------  include/hash.h
| 15 +++++++++++++++
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/common/hash.c b/common/hash.c index 12d6759..ea1ec60
100644
--- a/common/hash.c
+++ b/common/hash.c
@@ -20,7 +20,7 @@
 #include <asm/io.h>
 #include <asm/errno.h>
-#ifdef CONFIG_CMD_SHA1SUM
+#ifdef CONFIG_SHA1
I'm still not sure about this. I suspect this will bloat the code for boards
that use CONFIG_SHA1 (most) but not CONFIG_CMD_SHA1SUM. You could check that,
but I went through some contortions to make sure that the hash API was not
compiled in when not needed.
Since we will be using this API now in RSA checksum, defining CONFIG_SHA1 should allow the compilation of this structure. Asking user to enable CONFIG_CMD_SHA1SUM for using rsa-checksum doesn’t look right. Please suggest.
...
...
static int hash_init_sha1(struct hash_algo *algo, void **ctxp)  {
        sha1_context *ctx = malloc(sizeof(sha1_context)); @@ -125,12
+125,7 @@ static struct hash_algo hash_algo[] = {
                CHUNKSZ_SHA256,
        },
 #endif

  /*


   * This is CONFIG_CMD_SHA1SUM instead of CONFIG_SHA1 since



otherwise
...

   * it bloats the code for boards which use SHA1 but not the 'hash'


   * or 'sha1sum' commands.


   */



This is the comment referring to the above.
Is it possible to leave this logic as it is?
...
-#ifdef CONFIG_CMD_SHA1SUM
+#ifdef CONFIG_SHA1
        {
                "sha1",
                SHA1_SUM_LEN,
@@ -140,7 +135,6 @@ static struct hash_algo hash_algo[] = {
                hash_update_sha1,
                hash_finish_sha1,
        },
-#define MULTI_HASH
 #endif
 #ifdef CONFIG_SHA256
        {
@@ -152,7 +146,6 @@ static struct hash_algo hash_algo[] = {
                hash_update_sha256,
                hash_finish_sha256,
        },
-#define MULTI_HASH
 #endif
        {
                "crc32",
@@ -165,6 +158,10 @@ static struct hash_algo hash_algo[] = {
        },
 };
+#if defined(CONFIG_SHA256) || defined(CONFIG_CMD_SHA1SUM) #define
+MULTI_HASH #endif



#if defined(CONFIG_HASH_VERIFY) || defined(CONFIG_CMD_HASH)  #define
MULTI_HASH  #endif @@ -311,6 +308,24 @@ int hash_lookup_algo(const
char *algo_name, struct hash_algo **algop)
        return -EPROTONOSUPPORT;
 }
+int hash_progressive_lookup_algo(const char *algo_name,

                           struct hash_algo **algop) {


  int i;



  for (i = 0; i < ARRAY_SIZE(hash_algo); i++) {


          if (!strcmp(algo_name, hash_algo[i].name)) {


                  if (hash_algo[i].hash_init) {


                          *algop = &hash_algo[i];


                          return 0;


                  }


          }


  }



  debug("Unknown hash algorithm '%s'\n", algo_name);


  return -EPROTONOSUPPORT;



+}



void hash_show(struct hash_algo *algo, ulong addr, ulong len, uint8_t
*output)  {
        int i;
diff --git a/include/hash.h b/include/hash.h index d8ec4f0..059f84e
100644
--- a/include/hash.h
+++ b/include/hash.h
@@ -128,6 +128,21 @@ int hash_block(const char *algo_name, const void
*data, unsigned int len,  int hash_lookup_algo(const char *algo_name,
struct hash_algo **algop);
/**


hash_progressive_lookup_algo() - Look up the hash_algo struct with



progressive
...


                            hash support for an algorithm





Try to get that on one line if you can.
...






The function returns the pointer to the struct or -EPROTONOSUPPORT



+if the


algorithm is not available with progressive hash support.







@algo_name: Hash algorithm to look up



@algop: Pointer to the hash_algo struct if found







@return 0 if ok, -EPROTONOSUPPORT for an unknown algorithm.


*/

+int hash_progressive_lookup_algo(const char *algo_name,

                           struct hash_algo **algop);




+/**

hash_show() - Print out a hash algorithm and value

You will get a message like this (without a newline at the end):

--
1.8.1.4
Regards,
Simon
Regards,
Ruchika

    