Re: [PATCH 4/9] staging: wilc1000: free memory allocated in add wep key functions

[Ajay Singh <ajay.kathat@microchip.com>]

Hi Dan,

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