Hi Dan,
On Mon, 26 Mar 2018 11:17:48 +0300
Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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
