[libvirt] [PATCH 3/6] util: string: Introduce virStringEncodeBase64

[Peter Krempa]

On Fri, May 13, 2016 at 14:01:32 -0400, John Ferlan wrote:
> On 05/13/2016 12:04 PM, Peter Krempa wrote:
> > Add a new helper that sanitizes error semantics of base64_encode_alloc.
> > ---
> >  src/conf/virsecretobj.c           |  7 ++-----
> >  src/libvirt_private.syms          |  1 +
> >  src/qemu/qemu_agent.c             |  6 ++----
> >  src/storage/storage_backend_rbd.c | 17 ++++-------------
> >  src/util/virstring.c              | 24 ++++++++++++++++++++++++
> >  src/util/virstring.h              |  2 ++
> >  tools/virsh-secret.c              |  6 +++---
> >  7 files changed, 38 insertions(+), 25 deletions(-)
> > 
> 
> probably means #include "base64.h" is no longer necessary in places
> where the call has been removed.

Except for places that also call base64_decode_alloc.


[...]

> > +char *
> > +virStringEncodeBase64(const uint8_t *buf, size_t buflen)
> > +{
> > +    char *ret;
> > +
> > +    base64_encode_alloc((const char *) buf, buflen, &ret);
> > +    if (!ret) {
> > +        virReportOOMError();
> > +        return NULL;
> > +    }
> 
> Don't think the return NULL would be necessary...  just return ret

It's the same. Return ret saves one byte in the source, but is more
explicit in seeing what is going to be returned.

> > +
> > +    return ret;
> > +}
> > diff --git a/src/util/virstring.h b/src/util/virstring.h
> > index fd2868a..6bc2726 100644
> > --- a/src/util/virstring.h
> > +++ b/src/util/virstring.h

[...]

> > @@ -265,9 +266,8 @@ cmdSecretGetValue(vshControl *ctl, const vshCmd *cmd)
> >      if (value == NULL)
> >          goto cleanup;
> > 
> > -    base64_encode_alloc((char *)value, value_size, &base64);
> > -    memset(value, 0, value_size);
> > -    VIR_FREE(value);
> 
> Lost the VIR_FREE(value).... Could be VIR_DISPOSE(value) too.

Indeed. Also in the cleanup label.

> 
> 
> John
> > +    if (!(base64 = virStringEncodeBase64(value, value_size)))
> > +        goto cleanup;
> > 
> >      if (base64 == NULL) {
> >          vshError(ctl, "%s", _("Failed to allocate memory"));

Also this is no longer needed.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160516/cb80d1bf/attachment-0001.sig>