Re: [PATCH v4 02/15] crypto/hash-glib: Implement new hash API

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Aug 07, 2024 at 07:51:09PM +0000, Alejandro Zeise wrote:
> Implements the new hashing API in the GLib hash driver.
> Supports creating/destroying a context, updating the context
> with input data and obtaining an output hash.
> 
> Signed-off-by: Alejandro Zeise <alejandro.zeise@seagate.com>
> ---
>  crypto/hash-glib.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/crypto/hash-glib.c b/crypto/hash-glib.c
> index 82de9db705..9f4490762a 100644
> --- a/crypto/hash-glib.c
> +++ b/crypto/hash-glib.c
> @@ -1,6 +1,7 @@
>  /*
>   * QEMU Crypto hash algorithms
>   *
> + * Copyright (c) 2024 Seagate Technology LLC and/or its Affiliates
>   * Copyright (c) 2016 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -95,6 +96,81 @@ qcrypto_glib_hash_bytesv(QCryptoHashAlgorithm alg,
>  }
>  
>  
> +static
> +QCryptoHash *qcrypto_glib_hash_new(QCryptoHashAlgorithm alg,
> +                                   Error **errp)
> +{
> +    QCryptoHash *hash = NULL;
> +
> +    if (!qcrypto_hash_supports(alg)) {
> +        error_setg(errp,
> +                   "Unknown hash algorithm %d",
> +                   alg);

This check should be removed from all the drivers, since we
can do it once in qcrypto_hash_new.

> +    } else {
> +        hash = g_new(QCryptoHash, 1);
> +        hash->alg = alg;
> +        hash->opaque = g_checksum_new(qcrypto_hash_alg_map[alg]);
> +    }
> +
> +    return hash;
> +}
> +
> +static
> +void qcrypto_glib_hash_free(QCryptoHash *hash)
> +{
> +    if (hash->opaque) {
> +        g_checksum_free((GChecksum *) hash->opaque);

The cast to "(GChecksum *)" is not required since 'opaque' is 'void *'
which auto-casts to anything in C, and we don't need to support C++
compilers.

> +    }
> +
> +    g_free(hash);
> +}
> +
> +
> +static
> +int qcrypto_glib_hash_update(QCryptoHash *hash,
> +                             const struct iovec *iov,
> +                             size_t niov,
> +                             Error **errp)
> +{
> +    GChecksum *ctx = hash->opaque;
> +
> +    for (int i = 0; i < niov; i++) {
> +        g_checksum_update(ctx, iov[i].iov_base, iov[i].iov_len);
> +    }
> +
> +    return 0;
> +}
> +
> +static
> +int qcrypto_glib_hash_finalize(QCryptoHash *hash,
> +                               uint8_t **result,
> +                               size_t *result_len,
> +                               Error **errp)
> +{
> +    int ret;
> +    GChecksum *ctx = hash->opaque;
> +
> +    ret = g_checksum_type_get_length(qcrypto_hash_alg_map[hash->alg]);
> +    if (ret < 0) {
> +        error_setg(errp, "%s",
> +                   "Unable to get hash length");
> +        *result_len = 0;
> +        ret = -1;

Just do an immediate 'return -1' here, and avoid the
'} else {' clause.

> +    } else {
> +        *result_len = ret;
> +        *result = g_new(uint8_t, *result_len);
> +
> +        g_checksum_get_digest(ctx, *result, result_len);
> +        ret = 0;
> +    }
> +
> +    return ret;
> +}
> +
>  QCryptoHashDriver qcrypto_hash_lib_driver = {
>      .hash_bytesv = qcrypto_glib_hash_bytesv,
> +    .hash_new      = qcrypto_glib_hash_new,
> +    .hash_update   = qcrypto_glib_hash_update,
> +    .hash_finalize = qcrypto_glib_hash_finalize,
> +    .hash_free     = qcrypto_glib_hash_free,
>  };
> -- 
> 2.34.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|