[libvirt] [PATCH 01/13] conf: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT

[Eric Blake]

On 03/07/2014 02:45 AM, Michal Privoznik wrote:
> This fixes a possible double free. In virNetworkAssignDef() if
> virBitmapNew() fails, then virNetworkObjFree(network) is called.
> However, with network->def pointing to actual @def. So if caller
> frees @def again, ...
> 
> Moreover, this fixes one possible memory leak too. In
> virInterfaceAssignDef() if appending to the list of interfaces
> fails, we ought to call virInterfaceObjFree() instead of bare
> VIR_FREE().
> 
> Although, in order to do that some array size variables needs
> to be turned into size_t rather than int.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/conf/domain_conf.c      | 120 +++++++++-----------------------------------
>  src/conf/domain_conf.h      |   4 +-
>  src/conf/interface_conf.c   |  26 +++-------
>  src/conf/interface_conf.h   |   2 +-
>  src/conf/network_conf.c     |  30 ++++-------
>  src/conf/network_conf.h     |   4 +-
>  src/conf/node_device_conf.c |  16 ++----
>  src/conf/node_device_conf.h |   2 +-
>  src/conf/nwfilter_conf.c    |  33 ++++--------
>  src/conf/nwfilter_conf.h    |   8 +--
>  src/conf/nwfilter_params.c  |  12 ++---
>  src/conf/nwfilter_params.h  |   2 +-
>  src/conf/object_event.c     |  14 +-----
>  src/conf/storage_conf.c     |  16 ++----
>  src/qemu/qemu_driver.c      |   4 +-
>  15 files changed, 76 insertions(+), 217 deletions(-)
> 

> @@ -900,14 +887,17 @@ virNetworkDNSHostDefParseXML(const char *networkName,
>          if (cur->type == XML_ELEMENT_NODE &&
>              xmlStrEqual(cur->name, BAD_CAST "hostname")) {
>                if (cur->children != NULL) {
> -                  if (VIR_REALLOC_N(def->names, def->nnames + 1) < 0)
> -                      goto error;
> -                  def->names[def->nnames++] = (char *)xmlNodeGetContent(cur);
> -                  if (!def->names[def->nnames - 1]) {
> +                  char *name = (char *) xmlNodeGetContent(cur);
> +
> +                  if (!name) {
>                        virReportError(VIR_ERR_XML_DETAIL,
>                                       _("Missing hostname in network '%s' DNS HOST record"),
>                                       networkName);
>                    }
> +                  if (VIR_APPEND_ELEMENT(def->names, def->nnames, name) < 0) {
> +                      VIR_FREE(name);
> +                      goto error;
> +                  }

Pre-existing problem - if the xmlNodeGetContent failed, we reported the
error but didn't 'goto error'.  Your new code still has the problem; you
need to add a 'goto error' in the 'if (!name)' block.

Amazing how much more compact this is.  ACK with the error fix.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140307/5ee3e183/attachment-0001.sig>