Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1250945 > unrolled thread

[PATCH v2 0/3] staging: code cleanup

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/3] staging: code cleanup Luis de Bethencourt <luisbg@osg.samsung.com> - 2015-10-19 19:20 +0200
    Re: [PATCH v2 1/3] staging: rtl8712: Remove boolean comparisons Dan Carpenter <dan.carpenter@oracle.com> - 2015-10-22 21:10 +0200
      Re: [PATCH v2 1/3] staging: rtl8712: Remove boolean comparisons Luis de Bethencourt <luisbg@osg.samsung.com> - 2015-10-24 14:00 +0200

#1250945 — [PATCH v2 0/3] staging: code cleanup

Hi,

I initially submitted a patch to remove all the explicit boolean comparisons
in rtl8712. These code changes included two instances where braces needed to be
added to clean the output of checkpatch.pl on this patch.

Mike Rapaport correctly pointed out those changes had to go in a separate
patch. Not wanting to only fix those two instances, but cover the whole
rtl8712, I've added a second patch fixing these.

The checkpatch.pl of the second patch included some checks about needing spaces
around operands. Including this fix as well for a nicer checkpatch.pl output.
Plus also making it cover all rtl8712.

In other words, these are a series of recursive cleanup changes.

Thanks for the review Mike,
Luis


Luis de Bethencourt (3):
  staging: rtl8712: Remove boolean comparisons
  staging: rtl8712: braces should be used on all arms
  staging: rtl8712: spaces preferred around operands

 drivers/staging/rtl8712/hal_init.c            |  11 +-
 drivers/staging/rtl8712/ieee80211.c           |  36 ++---
 drivers/staging/rtl8712/mlme_linux.c          |   2 +-
 drivers/staging/rtl8712/os_intfs.c            |   6 +-
 drivers/staging/rtl8712/rtl8712_cmd.c         |  27 ++--
 drivers/staging/rtl8712/rtl8712_efuse.c       | 138 ++++++++++---------
 drivers/staging/rtl8712/rtl8712_io.c          |   6 +-
 drivers/staging/rtl8712/rtl8712_led.c         | 187 +++++++++++++-------------
 drivers/staging/rtl8712/rtl8712_recv.c        |  53 ++++----
 drivers/staging/rtl8712/rtl8712_xmit.c        |  74 +++++-----
 drivers/staging/rtl8712/rtl871x_cmd.c         |  25 ++--
 drivers/staging/rtl8712/rtl871x_eeprom.c      |  22 +--
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 153 +++++++++++----------
 drivers/staging/rtl8712/rtl871x_ioctl_rtl.c   |  53 +++++---
 drivers/staging/rtl8712/rtl871x_ioctl_set.c   |  33 ++---
 drivers/staging/rtl8712/rtl871x_mlme.c        | 144 ++++++++++----------
 drivers/staging/rtl8712/rtl871x_mlme.h        |   2 +-
 drivers/staging/rtl8712/rtl871x_mp.c          |  23 ++--
 drivers/staging/rtl8712/rtl871x_mp_ioctl.c    |  38 +++---
 drivers/staging/rtl8712/rtl871x_pwrctrl.c     |   6 +-
 drivers/staging/rtl8712/rtl871x_recv.c        |  70 +++++-----
 drivers/staging/rtl8712/rtl871x_security.c    | 133 +++++++++---------
 drivers/staging/rtl8712/rtl871x_sta_mgt.c     |  10 +-
 drivers/staging/rtl8712/rtl871x_xmit.c        |  95 ++++++-------
 drivers/staging/rtl8712/usb_halinit.c         |  13 +-
 drivers/staging/rtl8712/usb_intf.c            |  17 +--
 drivers/staging/rtl8712/usb_ops.c             |  20 +--
 drivers/staging/rtl8712/usb_ops_linux.c       |  20 +--
 drivers/staging/rtl8712/xmit_linux.c          |   4 +-
 29 files changed, 753 insertions(+), 668 deletions(-)

-- 
2.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1254118 — Re: [PATCH v2 1/3] staging: rtl8712: Remove boolean comparisons

On Mon, Oct 19, 2015 at 06:14:29PM +0100, Luis de Bethencourt wrote:
> Boolean tests do not need explicit comparison to true or false.
> 
> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> ---
> diff --git a/drivers/staging/rtl8712/usb_ops_linux.c b/drivers/staging/rtl8712/usb_ops_linux.c
> index c940722..e33eeed 100644
> --- a/drivers/staging/rtl8712/usb_ops_linux.c
> +++ b/drivers/staging/rtl8712/usb_ops_linux.c
> @@ -266,7 +266,7 @@ u32 r8712_usb_read_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem)
>  	if (adapter->bDriverStopped || adapter->bSurpriseRemoved ||
>  	    adapter->pwrctrlpriv.pnp_bstop_trx)
>  		return _FAIL;
> -	if (!precvbuf->reuse == false || !precvbuf->pskb) {
> +	if (precvbuf->reuse || !precvbuf->pskb) {
>  		precvbuf->pskb = skb_dequeue(&precvpriv->free_recv_skb_queue);
>  		if (precvbuf->pskb != NULL)
>  			precvbuf->reuse = true;

You have transformed this faithfully, but my instinct says that the
original code is wrong.  It should be:

	if (!precvbuf->reuse || !precvbuf->pskb) {

I checked and usb_read_port() is implemented this way in
drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c.  Again I am going on
instinct and not a full understanding of the code, but I'm probably
correct.

Anyway, this is not related to the patch so we should fix it in a later
patch, but let's not forget.

TODO: rtl8712: fix a reversed condition in r8712_usb_read_port()

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1255171 — Re: [PATCH v2 1/3] staging: rtl8712: Remove boolean comparisons

On 22/10/15 20:05, Dan Carpenter wrote:
> On Mon, Oct 19, 2015 at 06:14:29PM +0100, Luis de Bethencourt wrote:
>> Boolean tests do not need explicit comparison to true or false.
>>
>> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
>> ---
>> diff --git a/drivers/staging/rtl8712/usb_ops_linux.c b/drivers/staging/rtl8712/usb_ops_linux.c
>> index c940722..e33eeed 100644
>> --- a/drivers/staging/rtl8712/usb_ops_linux.c
>> +++ b/drivers/staging/rtl8712/usb_ops_linux.c
>> @@ -266,7 +266,7 @@ u32 r8712_usb_read_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem)
>>  	if (adapter->bDriverStopped || adapter->bSurpriseRemoved ||
>>  	    adapter->pwrctrlpriv.pnp_bstop_trx)
>>  		return _FAIL;
>> -	if (!precvbuf->reuse == false || !precvbuf->pskb) {
>> +	if (precvbuf->reuse || !precvbuf->pskb) {
>>  		precvbuf->pskb = skb_dequeue(&precvpriv->free_recv_skb_queue);
>>  		if (precvbuf->pskb != NULL)
>>  			precvbuf->reuse = true;
> 
> You have transformed this faithfully, but my instinct says that the
> original code is wrong.  It should be:
> 
> 	if (!precvbuf->reuse || !precvbuf->pskb) {
> 
> I checked and usb_read_port() is implemented this way in
> drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c.  Again I am going on
> instinct and not a full understanding of the code, but I'm probably
> correct.
> 
> Anyway, this is not related to the patch so we should fix it in a later
> patch, but let's not forget.
> 
> TODO: rtl8712: fix a reversed condition in r8712_usb_read_port()
> 
> regards,
> dan carpenter
> 

Hi Dan,

Thank you for the review. I will study the code and make sure that your
intuition is correct, which initially it sounds to be.

Thanks,
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web