| Message ID | 1455103178-3959-1-git-send-email-write.harmandeep@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Harmandeep Kaur writes ("[PATCH v2] libxc: fix leak in xc_offline_page error path"):
> 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>
Thanks.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
It is conventional, when you post a v2, to include the
still-applicable formal review tags.
Sometimes it is a matter of judgement, to decide whether an Acked-by
or Reviewed-by, still stands. But in this case Dario said:
The code looks ok to me, so, with or without the subject/changelog
improvements:
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
So, he has clearly indicated that your revised version should be
tagged:
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
You would put that right after the Reviewed-by.
However, there is no need to resend your patch for this. It is normal
for a committer to search the thread for acks to the patch which
arrive after it was posted, and fold them in.
In this case since the Subject has changed, someone needs to make sure
that that search turns up the review, but this message of mine will
serve that purpose.
So, having said all that, thanks for your contribution :-).
Regards,
Ian.
On Wed, 2016-02-10 at 11:51 +0000, Ian Jackson wrote: > Harmandeep Kaur writes ("[PATCH v2] libxc: fix leak in xc_offline_page > error path"): > > 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> > > Thanks. > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> applied, including... > Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com> > > You would put that right after the Reviewed-by. ...this. FTR I was unsure if xc_unmap_domain_meminfo would be safe to call after a failed xc_map_domain_meminfo (which it should be, but with libxc you never can tell) and I think it is indeed safe, although the failure paths in xc_map_domain_meminfo are pretty complicated they do appear to always reset fields in the struct to the "free" state. > > > However, there is no need to resend your patch for this. It is normal > for a committer to search the thread for acks to the patch which > arrive after it was posted, and fold them in. > > In this case since the Subject has changed, someone needs to make sure > that that search turns up the review, but this message of mine will > serve that purpose. > > > So, having said all that, thanks for your contribution :-). > > Regards, > Ian.
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 */
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> --- v2: update commit message and changelog --- tools/libxc/xc_offline_page.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)