Hi Dan,

On Mon, 26 Mar 2018 11:17:48 +0300
Dan Carpenter <dan.carpen...@oracle.com> wrote:

> On Fri, Mar 23, 2018 at 08:38:53PM +0530, Ajay Singh wrote:

> We should "return result;" here otherwise we'll hang when we
> wait_for_completion().  This is the sort of bug why I always encourage
> people to keep the error path and success path separate (unless they
> both have to unlock or free the same resources).
> 

Yes, wait_for_completion() will hang for the error path. I have included
the changes in V2 patch series.

> 
> This code works, but it would look cleaner with "return result;".
> 
>       result = wilc_enqueue_cmd(&msg);
>       if (result) {
>               netdev_err(vif->ndev, "AP - WEP Key\n");
>               kfree(msg.body.key_info.attr.wep.key);
>               return result;
>       }
> 
>       wait_for_completion(&hif_drv->comp_test_key_block);
>       return 0;
> 
> I removed a blank line between the wilc_enqueue_cmd() and the error
> handling because they're very connected.  All the success path is at
> indent level one so you can just glance at the function and see what
> it's supposed to do in the normal case.  The error handling is self
> contained at indent level two.
> 

I will send the updated patch by modifying the code as suggested.


Regards,
Ajay

Reply via email to