Re: [Qemu-devel] [PATCH v2 for-2.10 13/18] crypto: cipher: add afalg-backend cipher support

[Gonglei (Arei)]

> 
> 
> On 2017/4/21 20:59, Gonglei (Arei) wrote:
> 
> >
> >> -----Original Message-----
> >> From: longpeng
> >> Sent: Monday, April 17, 2017 9:33 AM
> >> To: address@hidden
> >> Cc: Gonglei (Arei); Huangweidong (C); address@hidden;
> >> address@hidden; address@hidden; address@hidden;
> longpeng
> >> Subject: [PATCH v2 for-2.10 13/18] crypto: cipher: add afalg-backend cipher
> >> support
> >>
> >> Adds afalg-backend cipher support: introduces some private APIs
> >> firstly, and then intergrates them into qcrypto_cipher_afalg_driver.
> >>
> >> Signed-off-by: Longpeng(Mike) <address@hidden>
> >> ---
> >>  crypto/Makefile.objs       |   1 +
> >>  crypto/afalgpriv.h         |   9 ++
> >>  crypto/cipher-afalg.c      | 225
> >> +++++++++++++++++++++++++++++++++++++++++++++
> >>  crypto/cipher.c            |  28 +++++-
> >>  crypto/cipherpriv.h        |  11 +++
> >>  include/crypto/cipher.h    |   8 ++
> >>  tests/test-crypto-cipher.c |  10 +-
> >>  7 files changed, 290 insertions(+), 2 deletions(-)
> >>  create mode 100644 crypto/cipher-afalg.c
> 
> >> +
> >> +static char *
> >> +qcrypto_afalg_cipher_format_name(QCryptoCipherAlgorithm alg,
> >> +                                 QCryptoCipherMode mode,
> >> +                                 Error **errp)
> >> +{
> >> +    char *name;
> >> +    const char *alg_name = NULL;
> >> +    const char *mode_name = NULL;
> >
> > Superfluous initialization.
> 
> 
> Ok.
> 
> >
> >> +    int ret;
> >> +
> 
> >> +
> >> +    mode_name = QCryptoCipherMode_lookup[mode];
> >> +
> >> +    name = (char *)g_new0(int8_t, SALG_NAME_LEN_MAX);
> >
> > s/ (char *)g_new0(int8_t, SALG_NAME_LEN_MAX)/g_new0(char,
> SALG_NAME_LEN_MAX)/
> >
> 
> 
> Ok.
> 
> >
> >> +    ret = snprintf(name, SALG_NAME_LEN_MAX, "%s(%s)", mode_name,
> >> +                   alg_name);
> >> +    if (ret < 0 || ret >= SALG_NAME_LEN_MAX) {
> 
> >> +
> >> +QCryptoAFAlg *
> >> +qcrypto_afalg_cipher_ctx_new(QCryptoCipherAlgorithm alg,
> >> +                             QCryptoCipherMode mode,
> >> +                             const uint8_t *key,
> >> +                             size_t nkey, Error **errp)
> >> +{
> >> +    QCryptoAFAlg *afalg;
> >> +    size_t except_niv = 0;
> >
> > Doesn't need to initialize it.
> >
> 
> 
> Ok.
> 
> >> +    char *name;
> >> +
> >> +    name = qcrypto_afalg_cipher_format_name(alg, mode, errp);
> >> +    if (!name) {
> >> +        return NULL;
> >> +    }
> >> +
> >> +    afalg = qcrypto_afalg_comm_alloc(AFALG_TYPE_CIPHER, name, errp);
> >> +    if (!afalg) {
> >> +        goto error;
> >
> > Leak memory pointed by name.
> >
> 
> 
> It won't.
> 
> If failed, the control flow is error->cleanup->return, and 'name' will be 
> freed
> in 'cleanup'.
> 
> If this method success, then the control flow is cleanup->return, 'name' will
> also be freed.
> 
I see. I suggest you can store the name pointer in afalg, then its easily to 
free
it in qcrypto_afalg_comm_free(), and we can get enough information about
the alg/mode if needed.

Thanks,
-Gonglei