Hi Tobias,
On Thu, Aug 14, 2014 at 12:11 PM, Tobias Klauser <tklauser@xxxxxxxxxx> wrote:
> 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
Thanks, I have sent a patch now for break and goto exit part. I will try looking
into functionalities, as you said will check if hardware required for
testing too.
In v3 of the PATCH i have made a mistake in formatting the patch. I have sent
only changes compare to my local commit (PATCH RESEND one), sorry for that.
Thanks
--
Regards
Jeshwanth Kumar N K
Bangalore, India
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
