[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Resolve memory leak found by valgrind



On 05/31/2013 09:54 AM, John Ferlan wrote:
> Commit '6afdfc8e' adjusted the exit and error paths to go through the error
> and cleanup labels, but neglected to remove the return ret prior to cleanup.
> Also noted the 'type' xml string fetch was never checked for NULL which
> could lead to some interesting results.
> ---
>  src/conf/storage_conf.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index f0ea41d..94eb69a 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -834,6 +834,12 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
>      }
>  
>      type = virXPathString("string(./@type)", ctxt);
> +    if (type == NULL) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("storage pool missing type attribute"));
> +        goto error;
> +    }
> +
>      if ((ret->type = virStoragePoolTypeFromString(type)) < 0) {
>          virReportError(VIR_ERR_XML_ERROR,
>                         _("unknown storage pool type %s"), type);
> @@ -956,8 +962,6 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
>              goto error;
>      }
>  
> -    return ret;
> -
>  cleanup:
>      VIR_FREE(uuid);
>      VIR_FREE(type);
> 

Peter also pointed out I was missing the "%s" in the new type check
error path. Something syntax-check would have caught (of course I had
run that on my other patch sent earlier, but forgot to on this one, sigh).

In any case, with the "%s" added, syntax-check passing, and rerun of
valgrind, I pushed.

Thanks,

John


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]