Re: [PATCH v3] staging: wlan-ng: prism2mgmt.c Fix break not useful

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 Tobias,

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?

If there is no 2nd value of return, this can be made as void function?
 or need to understand the logic behind?

And I didn't find it is checking P80211ENUM_resultcode_refused
anywhere in the code.

Thanks


-- 
Regards
Jeshwanth Kumar N K
Bangalore, India
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel





[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux