Re: [Xen-devel] [PATCH] 'dom' error handled, by raising ENOMEM if failure occurs. 'xs_fd' leak handled in success path.

[Lasya Venneti <comethalley61@xxxxxxxxx>]

On 27 October 2015 at 16:37, George Dunlap <george.dunlap@xxxxxxxxxx> wrote:

On 27/10/15 11:04, Lasya Venneti wrote:
> On 27 October 2015 at 16:25, Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> 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

I will definitely go through this thoroughly. Thank you for the resource.

Regards

Lasya V

Â-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 <comethalley61@xxxxxxxxx>
>>> ---
>>>Â 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
>
>> --
>> <<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)
>>
>>
>

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