Re: [Xen-devel] [PATCH 09/19] xen: lock target domain in do_domctl common code

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 11/20/2012 11:40 AM, Jan Beulich wrote:
>>>> On 19.11.12 at 16:20, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
>> On 11/19/2012 04:24 AM, Jan Beulich wrote:
>>>>>> On 16.11.12 at 19:28, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
>>>> @@ -458,6 +443,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
>> u_domctl)
>>>>          if ( IS_ERR(d) )
>>>>          {
>>>>              ret = PTR_ERR(d);
>>>> +            d = NULL;
>>>
>>> Considering that in the common code you already set d to NULL,
>>> is there a specific reason why you do so again here ...
>>>
>>>>              break;
>>>>          }
>>>>  
>>>> @@ -469,39 +455,28 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
>> u_domctl)
>>>>          op->domain = d->domain_id;
>>>>          if ( copy_to_guest(u_domctl, op, 1) )
>>>>              ret = -EFAULT;
>>>> +        d = NULL;
>>>
>>> ... and here?
>>>
>>> Same further down for XEN_DOMCTL_getdomaininfo.
>>>
>>> Jan
>>>
>>>>      }
>>>>      break;
>>>>  
>>>
>>>
>>>
>>
>> This avoids unlocking the domain when it hasn't been locked (at the
>> end of the function at domctl_out_unlock) or trying to unlock a
>> ERR_PTR value.
> 
> Sorry, this doesn't explain why d needs to be set to NULL twice.
> 
> Jan
> 

Maybe I misunderstood you: do you think one of the two assignments is
redundant? They are not, since the above is immediately followed by a
break, which jumps out of the switch statement to domctl_lock_release(),
and does not hit the second assignment.

-- 
Daniel De Graaf
National Security Agency

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