diff mbox

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

Message ID 1494356693-13190-1-git-send-email-sstabellini@kernel.org
State New
Headers show

Commit Message

Stefano Stabellini May 9, 2017, 7:04 p.m. UTC
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(+)

Comments

Eric Blake May 9, 2017, 7:20 p.m. UTC | #1
On 05/09/2017 02:04 PM, Stefano Stabellini 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(+)

Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake May 9, 2017, 7:28 p.m. UTC | #2
On 05/09/2017 02:20 PM, Eric Blake wrote:
> On 05/09/2017 02:04 PM, Stefano Stabellini 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(+)
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

By the way, you forgot to send a 0/3 cover letter (at least, there was
no In-Reply-To: header in your 1/3 mail).  That makes it harder to
automate handling of your series; I was going to reply to the series as
a whole.

While it is inconvenient for human readers, it is even worse for some of
the automated patch tooling we have that expects cover letters for any
multi-patch series.  More patch submission tips at
http://wiki.qemu.org/Contribute/SubmitAPatch include how to use 'git
config' to automate the creation of a cover letter.
Stefano Stabellini May 9, 2017, 7:47 p.m. UTC | #3
On Tue, 9 May 2017, Eric Blake wrote:
> On 05/09/2017 02:20 PM, Eric Blake wrote:
> > On 05/09/2017 02:04 PM, Stefano Stabellini 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(+)
> > 
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> By the way, you forgot to send a 0/3 cover letter (at least, there was
> no In-Reply-To: header in your 1/3 mail).  That makes it harder to
> automate handling of your series; I was going to reply to the series as
> a whole.
> 
> While it is inconvenient for human readers, it is even worse for some of
> the automated patch tooling we have that expects cover letters for any
> multi-patch series.  More patch submission tips at
> http://wiki.qemu.org/Contribute/SubmitAPatch include how to use 'git
> config' to automate the creation of a cover letter.

Sorry about that, and thanks for the review.
Greg Kurz May 9, 2017, 8:29 p.m. UTC | #4
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);
diff mbox

Patch

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);
         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);