From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH] 'dom' error handled, by raising ENOMEM if failure occurs. 'xs_fd' leak handled in success path. Date: Tue, 27 Oct 2015 11:07:21 +0000 Message-ID: <562F5AE9.10009@citrix.com> References: <1445984373-8087-1-git-send-email-comethalley61@gmail.com> <1445943305.2937.99.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Zr26C-0002S9-TB for xen-devel@lists.xenproject.org; Tue, 27 Oct 2015 11:07:29 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Lasya Venneti , Dario Faggioli Cc: xen-devel@lists.xenproject.org, Wei Liu , Ian Campbell , Lars Kurth List-Id: xen-devel@lists.xenproject.org On 27/10/15 11:04, Lasya Venneti wrote: > On 27 October 2015 at 16:25, Dario Faggioli > wrote: > >> On Wed, 2015-10-28 at 03:49 +0530, Lasya wrote: >> >> The description of the patch, that you previously put in a separate >> email, should live here. >> > Oh, I had thought no cover letter was needed. Hence, I didn't put in a > description, I shall add it in v3. :) Please also read carefully the section titled "Making good patches" in the Submitting Xen Project Patches wiki page [1]. [1] http://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches -George > >> >> Also, this is the second version of a patch. That should be evident >> from the subject line (something like "[PATCH v2]"). >> >> And still about the subject line, it's a bit long. It must contain a >> very quick hint at what the patch is about, then all the details go in >> the patch description (also called changelog), i.e., right in here. :-) >> > The subject line was picked up from the commit message, I shall put a > concise one this time. > >> >>> Signed-off-by: Lasya Venneti >>> --- >>> tools/xenstore/init-xenstore-domain.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/tools/xenstore/init-xenstore-domain.c >>> b/tools/xenstore/init-xenstore-domain.c >>> index 0d12169..b413b09 100644 >>> --- a/tools/xenstore/init-xenstore-domain.c >>> +++ b/tools/xenstore/init-xenstore-domain.c >>> @@ -42,6 +42,10 @@ static int build(xc_interface *xch, int argc, >>> char** argv) >>> snprintf(cmdline, 512, "--event %d --internal-db", rv); >>> >>> dom = xc_dom_allocate(xch, cmdline, NULL); >>> + if(dom==NULL) { >>> + rv=ENOMEM; >>> + goto err; >>> + } >>> >> A lot of spaces are missing... >> >>> rv = xc_dom_kernel_file(dom, argv[1]); >> ^ ^ >> | | >> ----------------------------------------------- >> | >> Just, for instance, compare what you added with this: --- >> >>> if (rv) goto err; >> ^ >> | >> -------- >> | >> And with this: --- >> >>> @@ -70,7 +74,8 @@ static int build(xc_interface *xch, int argc, >>> char** argv) >>> if (rv) goto err; >>> rv = xc_domain_unpause(xch, domid); >>> if (rv) goto err; >>> - >>> + >>> >> > Dario, I wasn't look at the individual spaces, just the line breaks. Will > be careful this time. > >> What happened here? >> >>> + close(xs_fd); >>> return 0; >>> >> Also, Wei suggested to send two patches, one for fixing the error >> handling for xc_dom_allocate(), and another one for stopping leaking >> the fd on the success path. >> >> Regards, >> Dario >> > Okay. I shall send them in as two patches. > > Thank you for being so patient with this. > Regards > Lasya V > >> -- >> <> (Raistlin Majere) >> ----------------------------------------------------------------- >> Dario Faggioli, Ph.D, http://about.me/dario.faggioli >> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) >> >> >