Re: [PATCH 1/4] xen/lib: Export additional sha256 functions

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 06/05/2025 3:05 pm, Andrew Cooper wrote:
> On 06/05/2025 2:56 pm, Frediano Ziglio wrote:
>> diff --git a/xen/include/xen/sha2.h b/xen/include/xen/sha2.h
>> index 47d97fbf01..ea8bad67e4 100644
>> --- a/xen/include/xen/sha2.h
>> +++ b/xen/include/xen/sha2.h
>> @@ -9,6 +9,16 @@
>>  
>>  #define SHA2_256_DIGEST_SIZE 32
>>  
>> +struct sha2_256_state {
>> +    uint32_t state[SHA2_256_DIGEST_SIZE / sizeof(uint32_t)];
>> +    uint8_t buf[64];
>> +    size_t count; /* Byte count. */
>> +};
>> +
>> +void sha2_256_init(struct sha2_256_state *s);
>> +void sha2_256_update(struct sha2_256_state *s, const void *msg,
>> +                     size_t len);
>> +void sha2_256_final(struct sha2_256_state *s, void *_dst);
>>  void sha2_256_digest(uint8_t digest[SHA2_256_DIGEST_SIZE],
>>                       const void *msg, size_t len);
> sha2_256_digest() is unlike the others as it holds sha2_256_state
> internally.  I'd suggest having all of the additions below this point,
> which group them more nicely.
>
> Can fix on commit.  Otherwise LGTM.

Not quite.  Now that sha2_256_final() is exported, it should have a
proper type for  _dst.  I've folded:

diff --git a/xen/include/xen/sha2.h b/xen/include/xen/sha2.h
index 0d55fe640431..cb30e8f8d77c 100644
--- a/xen/include/xen/sha2.h
+++ b/xen/include/xen/sha2.h
@@ -21,6 +21,7 @@ struct sha2_256_state {
 void sha2_256_init(struct sha2_256_state *s);
 void sha2_256_update(struct sha2_256_state *s, const void *msg,
                      size_t len);
-void sha2_256_final(struct sha2_256_state *s, void *_dst);
+void sha2_256_final(struct sha2_256_state *s,
+                    uint8_t digest[SHA2_256_DIGEST_SIZE]);
 
 #endif /* XEN_SHA2_H */
diff --git a/xen/lib/sha2-256.c b/xen/lib/sha2-256.c
index 896a257ed9b7..08ef7011a1c3 100644
--- a/xen/lib/sha2-256.c
+++ b/xen/lib/sha2-256.c
@@ -171,9 +171,9 @@ void sha2_256_update(struct sha2_256_state *s, const
void *msg,
     memcpy(s->buf + partial, msg, len);
 }
 
-void sha2_256_final(struct sha2_256_state *s, void *_dst)
+void sha2_256_final(struct sha2_256_state *s, uint8_t
digest[SHA2_256_DIGEST_SIZE])
 {
-    uint32_t *dst = _dst;
+    uint32_t *dst = (uint32_t *)digest;
     unsigned int i, partial = s->count & 63;
 
     /* Start padding */

in too, which is compatible with the rest of the series too.

~Andrew