On Mon, Jul 30, 2018 at 03:40:24PM +0530, Ajay Singh wrote:

Hi Dan,

On Mon, 30 Jul 2018 11:41:13 +0300
Dan Carpenter [off-list ref] wrote:

quoted

On Fri, Jul 20, 2018 at 04:35:24AM +0530, Ajay Singh wrote:

quoted

Hi Dan,

On Thu, 19 Jul 2018 12:27:44 +0300
Dan Carpenter [off-list ref] wrote:
  

quoted

On Thu, Jul 19, 2018 at 04:15:01AM +0530, Ajay Singh wrote:  

quoted

diff --git a/drivers/staging/wilc1000/wilc_wlan.c

b/drivers/staging/wilc1000/wilc_wlan.c index 85af365..8e71c28
100644 --- a/drivers/staging/wilc1000/wilc_wlan.c

+++ b/drivers/staging/wilc1000/wilc_wlan.c

@@ -850,13 +850,13 @@ static void

wilc_wlan_handle_isr_ext(struct wilc *wilc, u32 int_status) if
(wilc->rx_buffer) buffer = &wilc->rx_buffer[offset];
 		else
-			goto _end_;
+			goto end;    

This isn't related to your patch but this goto doesn't appear to
make any sort of sense.  I have no idea what was intended.
  

Thanks for pointing it out. I will include these changes in separate
patchset.

Yes, the position of goto label can be moved just before
wilc_wlan_handle_rxq(wilc), as 'ret' will always be '0' when goto
statement is executed.

Actually earlier there were few more goto statement in this function
and single label 'end' was used to handle for different cases. But
in previous cleanup patches those cases were removed.
Now this function can be further refactor by either moving
goto label before wilc_wlan_handle_rxq(wilc) or avoid goto use by
adding the rx_buffer validation along with size check.

i.e 

end:
    wilc_wlan_handle_rxq(wilc)


OR

	if (size > 0 && wilc->rx_buffer) {

	....
	}
    	wilc_wlan_handle_rxq(wilc)
  

Actually looking at it now, you could probably just remove the if
statement.  Hopefully wilc->rx_buffer is non-NULL at this point? Is
there really any need to call wilc_wlan_handle_rxq() when we haven't
called wilc_wlan_rxq_add()?

Yes, wilc->rx_buffer would be non NULL value as its only one time
allocated buffer. wilc_wlan_handle_rxq() was called without
wilc_wlan_rxq_add() just as a fail safe to ensure there are no pending
packets in the queue.

The only thing is wilc->quit can be set.  Otherwise if ->rx_buffer is
NULL it would just result in a NULL dereference.  (We are deep into
hypotheticals here because we're discussing impossible code).

regards,
dan carpenter