Re: [RFC v1 2/4] vfio-ccw: No need to call cp_free on an error in cp_init

Farhan Ali
2019-07-02T13:58:57Z

On 07/02/2019 04:42 AM, Cornelia Huck wrote:
> On Mon, 1 Jul 2019 12:23:44 -0400
> Farhan Ali <alifm@linux.ibm.com> wrote:
>
>> We don't set cp->initialized to true so calling cp_free
>> will just return and not do anything.
>>
>> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
>> ---
>> drivers/s390/cio/vfio_ccw_cp.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
>> index 5ac4c1e..cab1be9 100644
>> --- a/drivers/s390/cio/vfio_ccw_cp.c
>> +++ b/drivers/s390/cio/vfio_ccw_cp.c
>> @@ -647,8 +647,6 @@ int cp_init(struct channel_program *cp, struct device *mdev, union orb *orb)
>>
>> /* Build a ccwchain for the first CCW segment */
>> ret = ccwchain_handle_ccw(orb->cmd.cpa, cp);
>> - if (ret)
>> - cp_free(cp);
>
> Makes sense; hopefully ccwchain_handle_ccw() cleans up correctly on
> error :) (I think it does)
>

I have checked that it does as well, but wouldn't hurt if someone else
also glances over once again :)

> Maybe add a comment
>
> /* ccwchain_handle_ccw() already cleans up on error */
>
> so we don't stumble over this in the future?

Sure.

>
> (Also, does this want a Fixes: tag?)

This might warrant a fixes tag as well.
>
>>
>> if (!ret)
>> cp->initialized = true;
>
>