Re: [Qemu-devel] [PATCH v3 5/5] crypto: support multiple threads accessing one QCryptoBlock

[Alberto Garcia]

On Fri 07 Dec 2018 05:13:51 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> @@ -148,12 +154,97 @@ int qcrypto_block_encrypt(QCryptoBlock *block,
>  
>  QCryptoCipher *qcrypto_block_get_cipher(QCryptoBlock *block)
>  {
> -    return block->cipher;
> +    /* Ciphers should be accessed through pop/push method to be thread-safe.
> +     * Better, they should not be accessed externally at all (note, that
> +     * pop/push are static functions)
> +     * This function is used only in test with one thread (it's safe to skip
> +     * pop/push interface), so it's enough to assert it here:
> +     */
> +    assert(block->n_ciphers <= 1);
> +    return block->ciphers ? block->ciphers[0] : NULL;

If this is only supposed to be called in test mode I think you can also
assert that g_test_initialized() is true.

And the same with qcrypto_block_get_ivgen() later in this patch.

> +int qcrypto_block_init_cipher(QCryptoBlock *block,
> +                              QCryptoCipherAlgorithm alg,
> +                              QCryptoCipherMode mode,
> +                              const uint8_t *key, size_t nkey,
> +                              size_t n_threads, Error **errp)
> +{
> +    size_t i;
> +
> +    assert(!block->ciphers && !block->n_ciphers && !block->n_free_ciphers);
> +
> +    block->ciphers = g_new0(QCryptoCipher *, n_threads);

You can use g_new() instead of g_new0() because you're anyway
overwriting all elements of the array.

But these are minor nits, the patchs looks good to me else.

Reviewed-by: Alberto Garcia <address@hidden>

Berto