Re: [Qemu-devel] [PATCH v3 3/3] crypto: use CPP for wrapper definitions in nettle

[Daniel P. Berrange]

On Thu, Jul 16, 2015 at 06:04:01PM +0200, Radim Krčmář wrote:
> It's horrible both ways and I prefer this one.
> 
> Signed-off-by: Radim Krčmář <address@hidden>

As you say its ugly either way, but I think the amount of duplicated
code pattern is not too bad, and IMHO it is slightly clearer to read
without the macro usage.

I won't object to merge if other people think this macro usage is worth
while, but I don't think there's compelling need for it.

> ---
>  v3: make wrappers 'static'
>  v2: new
> 
>  crypto/cipher-nettle.c | 47 +++++++++++++++++------------------------------
>  1 file changed, 17 insertions(+), 30 deletions(-)
> 
> diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
> index 3e3c4c9cbf2f..5a9d506560ae 100644
> --- a/crypto/cipher-nettle.c
> +++ b/crypto/cipher-nettle.c
> @@ -33,34 +33,21 @@ typedef const void * cipher_ctx_t;
>  typedef size_t       cipher_length_t;
>  #endif
>  
> -static nettle_cipher_func aes_encrypt_wrapper;
> -static nettle_cipher_func aes_decrypt_wrapper;
> -static nettle_cipher_func des_encrypt_wrapper;
> -static nettle_cipher_func des_decrypt_wrapper;
> +#define WRAP(cipher) \
> +    static nettle_cipher_func cipher##_wrapper; \
> +    static void cipher##_wrapper(cipher_ctx_t ctx, cipher_length_t length, \
> +                          uint8_t *dst, const uint8_t *src) \
> +    { \
> +        cipher(ctx, length, dst, src); \
> +    }
>  
> -static void aes_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
> -                         uint8_t *dst, const uint8_t *src)
> -{
> -    aes_encrypt(ctx, length, dst, src);
> -}
> +#define WRAPPED(cipher) \
> +    cipher##_wrapper
>  
> -static void aes_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
> -                         uint8_t *dst, const uint8_t *src)
> -{
> -    aes_encrypt(ctx, length, dst, src);
> -}
> -
> -static void des_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
> -                         uint8_t *dst, const uint8_t *src)
> -{
> -    des_encrypt(ctx, length, dst, src);
> -}
> -
> -static void des_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
> -                         uint8_t *dst, const uint8_t *src)
> -{
> -    des_decrypt(ctx, length, dst, src);
> -}
> +WRAP(aes_encrypt)
> +WRAP(aes_decrypt)
> +WRAP(des_encrypt)
> +WRAP(des_decrypt)
>  
>  typedef struct QCryptoCipherNettle QCryptoCipherNettle;
>  struct QCryptoCipherNettle {
> @@ -122,8 +109,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm 
> alg,
>          des_set_key(ctx->ctx_encrypt, rfbkey);
>          g_free(rfbkey);
>  
> -        ctx->alg_encrypt = des_encrypt_wrapper;
> -        ctx->alg_decrypt = des_decrypt_wrapper;
> +        ctx->alg_encrypt = WRAPPED(des_encrypt);
> +        ctx->alg_decrypt = WRAPPED(des_decrypt);
>  
>          ctx->niv = DES_BLOCK_SIZE;
>          break;
> @@ -137,8 +124,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm 
> alg,
>          aes_set_encrypt_key(ctx->ctx_encrypt, nkey, key);
>          aes_set_decrypt_key(ctx->ctx_decrypt, nkey, key);
>  
> -        ctx->alg_encrypt = aes_encrypt_wrapper;
> -        ctx->alg_decrypt = aes_decrypt_wrapper;
> +        ctx->alg_encrypt = WRAPPED(aes_encrypt);
> +        ctx->alg_decrypt = WRAPPED(aes_decrypt);
>  
>          ctx->niv = AES_BLOCK_SIZE;
>          break;

Reviewed-by: Daniel P. Berrange <address@hidden>

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|