On Fri, Jul 20, 2018 at 04:35:24AM +0530, Ajay Singh wrote:
> Hi Dan,
>
> On Thu, 19 Jul 2018 12:27:44 +0300
> Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> > On Thu, Jul 19, 2018 at 04:15:01AM +0530, Ajay Singh wrote:
> > > 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()?
regards,
dan carpenter
