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 <[email protected]>
Berto
