On 2014-08-13 at 21:11:19 +0200, Jeshwanth Kumar N K <jeshkumar555@xxxxxxxxx> wrote:
> On Wed, Aug 13, 2014 at 12:06 PM, Tobias Klauser <tklauser@xxxxxxxxxx>
> wrote:
>
> > On 2014-08-12 at 20:54:48 +0200, Jeshwanth Kumar N K <
> > jeshkumar555@xxxxxxxxx> wrote:
> > > removed goto label, and directly returning 0 not through goto.
> > >
> > > Signed-off-by: Jeshwanth Kumar N K <jeshkumar555@xxxxxxxxx>
> > > ---
> > > drivers/staging/wlan-ng/prism2mgmt.c | 14 ++++----------
> > > 1 file changed, 4 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/staging/wlan-ng/prism2mgmt.c
> > b/drivers/staging/wlan-ng/prism2mgmt.c
> > > index 5837b0e..9218399 100644
> > > --- a/drivers/staging/wlan-ng/prism2mgmt.c
> > > +++ b/drivers/staging/wlan-ng/prism2mgmt.c
> > > @@ -1107,8 +1107,7 @@ int prism2mgmt_wlansniff(wlandevice_t *wlandev,
> > void *msgp)
> > > if (wlandev->netdev->type == ARPHRD_ETHER) {
> > > msg->resultcode.data =
> > > P80211ENUM_resultcode_invalid_parameters;
> > > - result = 0;
> > > - goto exit;
> > > + return 0;
> > > }
> > > /* Disable monitor mode */
> > > result = hfa384x_cmd_monitor(hw, HFA384x_MONITOR_DISABLE);
> > > @@ -1166,8 +1165,7 @@ int prism2mgmt_wlansniff(wlandevice_t *wlandev,
> > void *msgp)
> > >
> > > netdev_info(wlandev->netdev, "monitor mode disabled\n");
> > > msg->resultcode.data = P80211ENUM_resultcode_success;
> > > - result = 0;
> > > - goto exit;
> > > + return 0;
> > > case P80211ENUM_truth_true:
> > > /* Disable the port (if enabled), only check Port 0 */
> > > if (hw->port_enabled[0]) {
> > > @@ -1312,17 +1310,13 @@ int prism2mgmt_wlansniff(wlandevice_t *wlandev,
> > void *msgp)
> > > }
> > >
> > > msg->resultcode.data = P80211ENUM_resultcode_success;
> > > - result = 0;
> > > - goto exit;
> > > + return 0;
> > > default:
> > > msg->resultcode.data =
> > P80211ENUM_resultcode_invalid_parameters;
> > > - result = 0;
> > > - goto exit;
> > > + return 0;
> > > }
> > >
> > > failed:
> > > msg->resultcode.data = P80211ENUM_resultcode_refused;
> > > result = 0;
> > > -exit:
> > > - return result;
> >
> > You will still need the return here for the cases where the 'failed'
> > label is the jump target. Also, this change leads to a compiler warning:
> >
> > drivers/staging/wlan-ng/prism2usb.c: In function ‘prism2mgmt_wlansniff’:
> > drivers/staging/wlan-ng/prism2mgmt.c:1322:1: warning: control reaches end
> > of non-void function [-Wreturn-type]
> >
> > In any case, resetting the return value to 0 makes the function always
> > return
> > successfully and seems to signal the error condition by setting
> > msg->resultcode.data. This looks a bit odd to me and also doesn't
> > correspond to
> > the documentation of the function's return value. It might be worthwile to
> > take
> > a closer look there as well.
> >
> > Thanks
> > Tobias
> >
Hi Jeshwanth
> Thanks, I have changed it in local now (not yet submitted). But yes as you
> said it's not returning 0 every time.
>
> In comment it's mentioned that "<0 success, but we're waiting for
> something to finish.". This means this function implementation is not yet
> finished?
I assume so, but I'm not really familiar with the workings of this
driver.
> If there is no 2nd value of return, this can be made as void function? or
> need to understand the logic behind?
I would leave it as is, unless you clearly understand the logic behind
it. I'd suggest for your current patch to just change the
return/goto/break part and then take care about the return value in a
separate patch once you figured out how the function should really
behave. You'll most probably also need the hardware in question then to
test your changes on.
> And I didn't find it is checking P80211ENUM_resultcode_refused anywhere in
> the code.
Hm yes, strage. So it seems the error conditions in this functions are
not handled at all except for the pr_debug messages.
Thanks
Tobias
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
