Re: [Qemu-devel] [PATCH v3 12/18] crypto: introduce some common functions for af_alg backend

[Daniel P. Berrange]

On Sat, Apr 22, 2017 at 03:20:21PM +0800, Longpeng(Mike) wrote:
> The AF_ALG socket family is the userspace interface for linux
> crypto API, this patch adds af_alg family support and some common
> functions for af_alg backend. It'll be used by afalg-backend crypto
> latter.
> 
> Signed-off-by: Longpeng(Mike) <address@hidden>
> ---
>  configure            |  21 +++++++++
>  crypto/Makefile.objs |   1 +
>  crypto/afalg.c       | 118 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  crypto/afalgpriv.h   |  59 ++++++++++++++++++++++++++
>  4 files changed, 199 insertions(+)
>  create mode 100644 crypto/afalg.c
>  create mode 100644 crypto/afalgpriv.h

> +static bool
> +qcrypto_afalg_build_saddr(const char *type, const char *name,
> +                          struct sockaddr_alg *salg, Error **errp)
> +{
> +    salg->salg_family = AF_ALG;
> +
> +    if (qemu_strnlen(type, SALG_TYPE_LEN_MAX) == SALG_TYPE_LEN_MAX) {
> +        error_setg(errp, "Afalg type(%s) is larger than %d bytes",
> +                   type, SALG_TYPE_LEN_MAX);
> +        return false;
> +    }
> +
> +    if (qemu_strnlen(name, SALG_NAME_LEN_MAX) == SALG_NAME_LEN_MAX) {
> +        error_setg(errp, "Afalg name(%s) is larger than %d bytes",
> +                   name, SALG_NAME_LEN_MAX);
> +        return false;
> +    }

This is uneccessarily complicated - we can just use

   if (strlen(name) >= SALG_NAME_LEN_MAX) {
       ....
   }

> +    pstrcpy((char *)salg->salg_type, SALG_TYPE_LEN_MAX, type);
> +    pstrcpy((char *)salg->salg_name, SALG_NAME_LEN_MAX, name);
> +
> +    return true;
> +}


> +void qcrypto_afalg_comm_free(QCryptoAFAlg *afalg)
> +{
> +    if (afalg) {

Rather than indenting the entire method it is preferrable to
invert the condition and return immediately, eg

  if (!afalg) {
     return;
  }

> +        if (afalg->msg) {
> +            g_free(afalg->msg->msg_control);
> +            g_free(afalg->msg);
> +        }
> +
> +        if (afalg->name) {
> +            g_free(afalg->name);
> +        }
> +
> +        if (afalg->tfmfd != -1) {
> +            closesocket(afalg->tfmfd);
> +        }
> +
> +        if (afalg->opfd != -1) {
> +            closesocket(afalg->opfd);
> +        }
> +
> +        g_free(afalg);
> +    }
> +}


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 :|