Re: [Qemu-devel] [PATCH v2 1/3] xen/9pfs: fix two resource leaks on error paths, discovered by Coverity

[Greg Kurz <groug@kaod.org>]

[-- Attachment #1: Type: text/plain, Size: 1596 bytes --]

On Tue,  9 May 2017 12:04:51 -0700
Stefano Stabellini <sstabellini@kernel.org> wrote:

> CID: 1374836
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> CC: anthony.perard@citrix.com
> CC: groug@kaod.org
> CC: aneesh.kumar@linux.vnet.ibm.com
> ---
>  hw/9pfs/xen-9p-backend.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
> index 9c7f41a..a1fdede 100644
> --- a/hw/9pfs/xen-9p-backend.c
> +++ b/hw/9pfs/xen-9p-backend.c
> @@ -332,12 +332,14 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
>          str = g_strdup_printf("ring-ref%u", i);
>          if (xenstore_read_fe_int(&xen_9pdev->xendev, str,
>                                   &xen_9pdev->rings[i].ref) == -1) {
> +            g_free(str);
>              goto out;
>          }
>          g_free(str);

I would rather do something like:

    int ret;

    [...]

        str = g_strdup_printf("ring-ref%u", i);
        ret = xenstore_read_fe_int(&xen_9pdev->xendev, str,
                                   &xen_9pdev->rings[i].ref);
        g_free(str);
        if (ret ==  -1) {
            goto out;
        }

but this is a matter of taste and you own this code so:

Reviewed-by: Greg Kurz <groug@kaod.org>

>          str = g_strdup_printf("event-channel-%u", i);
>          if (xenstore_read_fe_int(&xen_9pdev->xendev, str,
>                                   &xen_9pdev->rings[i].evtchn) == -1) {
> +            g_free(str);
>              goto out;
>          }
>          g_free(str);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]