Change the return values of function r8712_setdatarate_cmd from _SUCCESS
and _FAIL to 0 and -ENOMEM respectively.
Change the return type of the function from u8 to int to reflect this.
Change the call site of the function to check for 0 instead of _SUCCESS.
(Checking that the return value != 0 is not necessary; the return value
itself can simply be passed into the conditional.)
Signed-off-by: Nishka Dasgupta <[email protected]>
---
drivers/staging/rtl8712/rtl871x_cmd.c | 8 ++++----
drivers/staging/rtl8712/rtl871x_cmd.h | 2 +-
drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c b/drivers/staging/rtl8712/rtl871x_cmd.c
index 05a78ac24987..e478c031f95f 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.c
+++ b/drivers/staging/rtl8712/rtl871x_cmd.c
@@ -242,7 +242,7 @@ u8 r8712_sitesurvey_cmd(struct _adapter *padapter,
return _SUCCESS;
}
-u8 r8712_setdatarate_cmd(struct _adapter *padapter, u8 *rateset)
+int r8712_setdatarate_cmd(struct _adapter *padapter, u8 *rateset)
{
struct cmd_obj *ph2c;
struct setdatarate_parm *pbsetdataratepara;
@@ -250,18 +250,18 @@ u8 r8712_setdatarate_cmd(struct _adapter *padapter, u8 *rateset)
ph2c = kmalloc(sizeof(*ph2c), GFP_ATOMIC);
if (!ph2c)
- return _FAIL;
+ return -ENOMEM;
pbsetdataratepara = kmalloc(sizeof(*pbsetdataratepara), GFP_ATOMIC);
if (!pbsetdataratepara) {
kfree(ph2c);
- return _FAIL;
+ return -ENOMEM;
}
init_h2fwcmd_w_parm_no_rsp(ph2c, pbsetdataratepara,
GEN_CMD_CODE(_SetDataRate));
pbsetdataratepara->mac_id = 5;
memcpy(pbsetdataratepara->datarates, rateset, NumRates);
r8712_enqueue_cmd(pcmdpriv, ph2c);
- return _SUCCESS;
+ return 0;
}
u8 r8712_set_chplan_cmd(struct _adapter *padapter, int chplan)
diff --git a/drivers/staging/rtl8712/rtl871x_cmd.h b/drivers/staging/rtl8712/rtl871x_cmd.h
index 262984c58efb..800216cca2f6 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.h
+++ b/drivers/staging/rtl8712/rtl871x_cmd.h
@@ -719,7 +719,7 @@ u8 r8712_joinbss_cmd(struct _adapter *padapter,
u8 r8712_disassoc_cmd(struct _adapter *padapter);
u8 r8712_setopmode_cmd(struct _adapter *padapter,
enum NDIS_802_11_NETWORK_INFRASTRUCTURE networktype);
-u8 r8712_setdatarate_cmd(struct _adapter *padapter, u8 *rateset);
+int r8712_setdatarate_cmd(struct _adapter *padapter, u8 *rateset);
u8 r8712_set_chplan_cmd(struct _adapter *padapter, int chplan);
u8 r8712_setbasicrate_cmd(struct _adapter *padapter, u8 *rateset);
u8 r8712_getrfreg_cmd(struct _adapter *padapter, u8 offset, u8 *pval);
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index b424b8436fcf..761e2ba68a42 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -1367,7 +1367,7 @@ static int r8711_wx_set_rate(struct net_device *dev,
datarates[i] = 0xff;
}
}
- if (r8712_setdatarate_cmd(padapter, datarates) != _SUCCESS)
+ if (r8712_setdatarate_cmd(padapter, datarates))
ret = -ENOMEM;
return ret;
}
--
2.19.1
Probably you sent this patch unintentionally. The subject doesn't make
any sort of sense. :P
On Fri, Jun 07, 2019 at 07:36:57PM +0530, Nishka Dasgupta wrote:
> Change the return values of function r8712_setdatarate_cmd from _SUCCESS
> and _FAIL to 0 and -ENOMEM respectively.
> Change the return type of the function from u8 to int to reflect this.
> Change the call site of the function to check for 0 instead of _SUCCESS.
> (Checking that the return value != 0 is not necessary; the return value
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> itself can simply be passed into the conditional.)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is obvious. No need to mention it in the commit message.
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index b424b8436fcf..761e2ba68a42 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -1367,7 +1367,7 @@ static int r8711_wx_set_rate(struct net_device *dev,
> datarates[i] = 0xff;
> }
> }
> - if (r8712_setdatarate_cmd(padapter, datarates) != _SUCCESS)
> + if (r8712_setdatarate_cmd(padapter, datarates))
> ret = -ENOMEM;
>
> return ret;
It would be better to write this like so:
ret = r8712_setdatarate_cmd(padapter, datarates);
if (ret)
return ret;
return 0;
Or you could write it like:
return r8712_setdatarate_cmd(padapter, datarates);
Which ever one you prefer is fine.
regards,
dan carpenter
On 07/06/19 7:45 PM, Dan Carpenter wrote:
> Probably you sent this patch unintentionally. The subject doesn't make
> any sort of sense. :P
So the problem with the subject line is that git send-email and vim (as
configured on my laptop) tend to line-wrap even the subject line. Since
I have two patches that do the same thing for different functions, I
felt I should have the driver and the function name in the subject line
(to avoid confusion between the patches and to allow for easy searching
later). But that doesn't leave enough space in the subject line for
"Change return values/type" or any other descriptive message. What
should I do?
> On Fri, Jun 07, 2019 at 07:36:57PM +0530, Nishka Dasgupta wrote:
>> Change the return values of function r8712_setdatarate_cmd from _SUCCESS
>> and _FAIL to 0 and -ENOMEM respectively.
>> Change the return type of the function from u8 to int to reflect this.
>> Change the call site of the function to check for 0 instead of _SUCCESS.
>> (Checking that the return value != 0 is not necessary; the return value
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> itself can simply be passed into the conditional.)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This is obvious. No need to mention it in the commit message.
Okay, I'll amend that.
>> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> index b424b8436fcf..761e2ba68a42 100644
>> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> @@ -1367,7 +1367,7 @@ static int r8711_wx_set_rate(struct net_device *dev,
>> datarates[i] = 0xff;
>> }
>> }
>> - if (r8712_setdatarate_cmd(padapter, datarates) != _SUCCESS)
>> + if (r8712_setdatarate_cmd(padapter, datarates))
>> ret = -ENOMEM;
>>
>> return ret;
>
>
> It would be better to write this like so:
>
> ret = r8712_setdatarate_cmd(padapter, datarates);
> if (ret)
> return ret;
>
> return 0;
>
> Or you could write it like:
>
> return r8712_setdatarate_cmd(padapter, datarates);
Okay, since this is the only point at which a return happens in this
function, I can do this.
> Which ever one you prefer is fine
Thanking you,
Nishka
> regards,
> dan carpenter
>
On Mon, Jun 10, 2019 at 10:02:27AM +0530, Nishka Dasgupta wrote:
> On 07/06/19 7:45 PM, Dan Carpenter wrote:
> > Probably you sent this patch unintentionally. The subject doesn't make
> > any sort of sense. :P
>
> So the problem with the subject line is that git send-email and vim (as
> configured on my laptop) tend to line-wrap even the subject line. Since I
> have two patches that do the same thing for different functions, I felt I
> should have the driver and the function name in the subject line (to avoid
> confusion between the patches and to allow for easy searching later). But
> that doesn't leave enough space in the subject line for "Change return
> values/type" or any other descriptive message. What should I do?
>
I don't really care.
[PATCH] staging: rtl8712: clean up r8712_setdatarate_cmd() return type
regards,
dan carpenter