Re: [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change

[Dan Carpenter]

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