[PATCH] libxc: Fix CID 1351225 resource leak

[PATCH] libxc: Fix CID 1351225 resource leak

[Harmandeep Kaur]

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
---
 tools/libxc/xc_offline_page.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c
index bc91d51..3248a34 100644
--- a/tools/libxc/xc_offline_page.c
+++ b/tools/libxc/xc_offline_page.c
@@ -504,7 +504,7 @@ int xc_exchange_page(xc_interface *xch, int domid, xen_pfn_t mfn)
     if ( xc_map_domain_meminfo(xch, domid, &minfo) )
     {
         PERROR("Could not map domain's memory information\n");
-        return -1;
+        goto failed;
     }
 
     /* For translation macros */
-- 
2.5.0

Re: [PATCH] libxc: Fix CID 1351225 resource leak

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1148 bytes --]

Hi Harmandeep,

Thanks for this patch.

On Wed, 2016-02-10 at 14:37 +0530, Harmandeep Kaur wrote:
>
In general, I think it would be best if the subject is a bit more
"explicative", and if you add a few words of changelog, here, above the
S-o-b.

In this case, this could be something like this.

Subject: "libxc: fix leak in xc_offline_page error path"

Changelog: "
Avoid leaking the mapping of the m2p in one of the possible failure
cases.

Coverity CID 1351225
"

> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
>
That being said, this case is very simple, so I'll leave it to the
tools maintainers to tell whether they want something like what I
described above in place or not.

The code looks ok to me, so, with or without the subject/changelog
improvements:

Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] libxc: Fix CID 1351225 resource leak

[Ian Campbell]

On Wed, 2016-02-10 at 10:28 +0100, Dario Faggioli wrote:
> Hi Harmandeep,
> 
> Thanks for this patch.
> 
> On Wed, 2016-02-10 at 14:37 +0530, Harmandeep Kaur wrote:
> > 
> In general, I think it would be best if the subject is a bit more
> "explicative", and if you add a few words of changelog, here, above the
> S-o-b.
> 
> In this case, this could be something like this.
> 
> Subject: "libxc: fix leak in xc_offline_page error path"
> 
> Changelog: "
> Avoid leaking the mapping of the m2p in one of the possible failure
> cases.
> 
> Coverity CID 1351225
> "
> 
> > Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> > 
> That being said, this case is very simple, so I'll leave it to the
> tools maintainers to tell whether they want something like what I
> described above in place or not.

I was going to comment the same, so yes please. The subject and commit
message should describe the actual change and the CID should simply be
referenced as Dario has in his example.

> 
> The code looks ok to me, so, with or without the subject/changelog
> improvements:
> 
> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
> 
> Regards,
> Dario

Re: [PATCH] libxc: Fix CID 1351225 resource leak

[Ian Jackson]

Dario Faggioli writes ("Re: [PATCH] libxc: Fix CID 1351225 resource leak"):
> That being said, this case is very simple, so I'll leave it to the
> tools maintainers to tell whether they want something like what I
> described above in place or not.

I agree that what you suggest would be better.  The committer could
fold in your suggestions at the time of commit, but it would be nicer
to have a formal v2 with an updated commit message.

> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

Thanks.

Ian.