Re: [PATCH v8 02/25] xen/xsplice: Hypervisor implementation of XEN_XSPLICE_op

[Ross Lagerwall <ross.lagerwall@citrix.com>]

On 04/25/2016 10:03 AM, Jan Beulich wrote:
>>>> On 22.04.16 at 20:06, <konrad.wilk@oracle.com> wrote:
>> On Wed, Apr 20, 2016 at 02:18:48PM +0100, Ross Lagerwall wrote:
>>> On 04/13/2016 10:09 PM, Konrad Rzeszutek Wilk wrote:
>>> snip
>>>> +static int xsplice_action(xen_sysctl_xsplice_action_t *action)
>>>> +{
>>>> +    struct payload *data;
>>>> +    char n[XEN_XSPLICE_NAME_SIZE];
>>>> +    int rc;
>>>> +
>>>> +    rc = verify_name(&action->name, n);
>>>> +    if ( rc )
>>>> +        return rc;
>>>> +
>>>> +    spin_lock(&payload_lock);
>>>> +
>>>> +    data = find_payload(n);
>>>> +    if ( IS_ERR_OR_NULL(data) )
>>>> +    {
>>>> +        spin_unlock(&payload_lock);
>>>> +
>>>> +        if ( !data )
>>>> +            return -ENOENT;
>>>> +
>>>> +        return PTR_ERR(data);
>>>> +    }
>>>
>>> I think you need something like: rc = -EINVAL;
>>> here. Otherwise running "xen-xsplice unload <patch>" on an applied patch
>>> succeeds but fails to do anything.
>>
>> /me nods. Fixed.
>
> I don't follow: There's no path out of the if()'s body, and a proper
> error indicator gets handed to the caller via the two return
> statements. Why would this need converting to -EINVAL?
>

I intended that line to be inserted after the if statement to set a 
default return value for the unhandled paths through the following 
switch(), not to change the if().

-- 
Ross Lagerwall

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