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

Groups > linux.kernel > #1445067 > unrolled thread

[PATCH 0/9] staging: ks7010: Fine-tuning for a SDIO card driver

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 0/9] staging: ks7010: Fine-tuning for a SDIO card driver SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-17 20:10 +0200
    [PATCH 2/9] staging: ks7010: Delete unnecessary assignments for  buffer variables SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-17 20:20 +0200
      Re: [PATCH 2/9] staging: ks7010: Delete unnecessary assignments for  buffer variables Wolfram Sang <wsa@the-dreams.de> - 2016-07-20 17:50 +0200
    [PATCH 1/9] staging: ks7010: Delete unnecessary checks before the  function call "kfree" SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-17 20:20 +0200
      Re: [PATCH 1/9] staging: ks7010: Delete unnecessary checks before  the function call "kfree" Wolfram Sang <wsa@the-dreams.de> - 2016-07-20 17:50 +0200
    [PATCH 3/9] staging: ks7010: Return directly after a failed kmalloc() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-17 20:20 +0200
      Re: [PATCH 3/9] staging: ks7010: Return directly after a failed  kmalloc() Julia Lawall <julia.lawall@lip6.fr> - 2016-07-17 21:00 +0200
        Re: [PATCH 3/9] staging: ks7010: Return directly after a failed  kmalloc() Wolfram Sang <wsa@the-dreams.de> - 2016-07-20 17:50 +0200
          Re: [PATCH 3/9] staging: ks7010: Return directly after a failed  kmalloc() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-20 20:50 +0200
            Re: [PATCH 3/9] staging: ks7010: Return directly after a failed  kmalloc() Wolfram Sang <wsa@the-dreams.de> - 2016-07-21 08:30 +0200
              Re: staging: ks7010: Return directly after a failed kmalloc() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-22 09:40 +0200
                Re: staging: ks7010: Return directly after a failed kmalloc() Wolfram Sang <wsa@the-dreams.de> - 2016-07-22 09:50 +0200
    [PATCH 7/9] staging: ks7010: Replace three printk() calls by pr_err() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-17 20:30 +0200
      Re: [PATCH 7/9] staging: ks7010: Replace three printk() calls by  pr_err() Joe Perches <joe@perches.com> - 2016-07-17 22:30 +0200
        Re: [PATCH 7/9] staging: ks7010: Replace three printk() calls by  pr_err() Wolfram Sang <wsa@the-dreams.de> - 2016-07-20 18:00 +0200
    [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable  "retval" SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-17 20:30 +0200
      Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the  variable "retval" Julia Lawall <julia.lawall@lip6.fr> - 2016-07-17 21:00 +0200
        Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the  variable "retval" Wolfram Sang <wsa@the-dreams.de> - 2016-07-20 18:00 +0200
          Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the  variable "retval" SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-20 20:50 +0200
            Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the  variable "retval" Wolfram Sang <wsa@the-dreams.de> - 2016-07-21 08:30 +0200
              Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the  variable "retval" Wolfram Sang <wsa@the-dreams.de> - 2016-07-21 09:30 +0200
              Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the  variable "retval" Julia Lawall <julia.lawall@lip6.fr> - 2016-07-21 09:30 +0200
                Re: staging: ks7010: Delete unnecessary uses of the variable "retval" SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-21 14:50 +0200
                  Re: staging: ks7010: Delete unnecessary uses of the variable  "retval" Julia Lawall <julia.lawall@lip6.fr> - 2016-07-21 15:00 +0200
                    Re: staging: ks7010: Delete unnecessary uses of the variable "retval" Wolfram Sang <wsa@the-dreams.de> - 2016-07-21 18:40 +0200
              Re: staging: ks7010: Delete unnecessary uses of the variable "retval" SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-21 15:40 +0200
                Re: staging: ks7010: Delete unnecessary uses of the variable "retval" Wolfram Sang <wsa@the-dreams.de> - 2016-07-21 18:40 +0200
                  Re: staging: ks7010: Delete unnecessary uses of the variable "retval" SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-21 23:30 +0200
                    Re: staging: ks7010: Delete unnecessary uses of the variable "retval" Wolfram Sang <wsa@the-dreams.de> - 2016-07-22 08:00 +0200
    [PATCH 8/9] staging: ks7010: Delete a variable in write_to_device() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-17 20:30 +0200
      Re: [PATCH 8/9] staging: ks7010: Delete a variable in  write_to_device() Wolfram Sang <wsa@the-dreams.de> - 2016-07-20 18:00 +0200
    [PATCH 4/9] staging: ks7010: Rename jump labels SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-17 20:30 +0200
      Re: [PATCH 4/9] staging: ks7010: Rename jump labels Wolfram Sang <wsa@the-dreams.de> - 2016-07-20 18:00 +0200
        Re: staging: ks7010: Rename jump labels SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-20 20:30 +0200
          Re: staging: ks7010: Rename jump labels Jean Delvare <jdelvare@suse.de> - 2016-07-20 23:20 +0200
            Re: staging: ks7010: Rename jump labels Wolfram Sang <wsa@the-dreams.de> - 2016-07-21 08:30 +0200
            Re: staging: ks7010: Rename jump labels "SF Markus Elfring" <elfring@users.sourceforge.net> - 2016-07-21 10:00 +0200
              Re: staging: ks7010: Rename jump labels Jean Delvare <jdelvare@suse.de> - 2016-07-25 14:40 +0200
            Re: staging: ks7010: Rename jump labels SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-21 17:40 +0200
              Re: staging: ks7010: Rename jump labels Jean Delvare <jdelvare@suse.de> - 2016-07-21 21:20 +0200
                Re: staging: ks7010: Rename jump labels SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-21 22:30 +0200
                  Re: staging: ks7010: Rename jump labels Jean Delvare <jdelvare@suse.de> - 2016-07-25 14:40 +0200
                    Re: staging: ks7010: Rename jump labels SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-25 18:20 +0200
                      Re: staging: ks7010: Rename jump labels Jean Delvare <jdelvare@suse.de> - 2016-07-25 23:10 +0200
    [PATCH 6/9] staging: ks7010: Delete unnecessary braces SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-17 20:30 +0200
      Re: [PATCH 6/9] staging: ks7010: Delete unnecessary braces Julia Lawall <julia.lawall@lip6.fr> - 2016-07-17 21:00 +0200
        Re: [PATCH 6/9] staging: ks7010: Delete unnecessary braces Wolfram Sang <wsa@the-dreams.de> - 2016-07-20 18:00 +0200
    [PATCH 9/9] staging: ks7010: Delete three unnecessary variable  initialisations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-17 20:40 +0200
      Re: [PATCH 9/9] staging: ks7010: Delete three unnecessary variable  initialisations Julia Lawall <julia.lawall@lip6.fr> - 2016-07-17 21:00 +0200
        Re: [PATCH 9/9] staging: ks7010: Delete three unnecessary variable  initialisations Wolfram Sang <wsa@the-dreams.de> - 2016-07-20 18:00 +0200
        Re: [PATCH 9/9] staging: ks7010: Delete three unnecessary variable  initialisations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-21 15:50 +0200
          Re: [PATCH 9/9] staging: ks7010: Delete three unnecessary variable  initialisations Julia Lawall <julia.lawall@lip6.fr> - 2016-07-21 16:00 +0200
            Re: staging: ks7010: Delete three unnecessary variable  initialisations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-21 16:20 +0200

Page 1 of 3  [1] 2 3  Next page →

#1445067 — [PATCH 0/9] staging: ks7010: Fine-tuning for a SDIO card driver

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 17 Jul 2016 19:54:32 +0200

Further update suggestions were taken into account
after a patch was applied from static source code analysis.

Markus Elfring (9):
  Delete unnecessary checks before the function call "kfree"
  Delete unnecessary assignments for buffer variables
  Return directly after a failed kmalloc()
  Rename jump labels
  Delete unnecessary uses of the variable "retval"
  Delete unnecessary braces
  Replace three printk() calls by pr_err()
  Delete a variable in write_to_device()
  Delete three unnecessary variable initialisations

 drivers/staging/ks7010/ks7010_sdio.c | 263 ++++++++++++++---------------------
 1 file changed, 107 insertions(+), 156 deletions(-)

-- 
2.9.1

[toc] | [next] | [standalone]

#1445069 — [PATCH 2/9] staging: ks7010: Delete unnecessary assignments for buffer variables

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 17 Jul 2016 13:38:46 +0200

A few variables were assigned a null pointer despite of the detail
that they were immediately reassigned by the following statement.
Thus remove such unnecessary assignments.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/ks7010/ks7010_sdio.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 7da6c84..3622fba 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -711,7 +711,6 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
 	int rc = 0;
 	int retval;
 	unsigned char *data_buf;
-	data_buf = NULL;
 
 	data_buf = kmalloc(sizeof(u32), GFP_KERNEL);
 	if (!data_buf) {
@@ -743,7 +742,7 @@ static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address,
 	int rc = 0;
 	int retval;
 	unsigned char *read_buf;
-	read_buf = NULL;
+
 	read_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL);
 	if (!read_buf) {
 		rc = 1;
@@ -776,8 +775,6 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv,
 	int length;
 	const struct firmware *fw_entry = NULL;
 
-	rom_buf = NULL;
-
 	/* buffer allocate */
 	rom_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL);
 	if (!rom_buf) {
-- 
2.9.1

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

#1447314 — Re: [PATCH 2/9] staging: ks7010: Delete unnecessary assignments for buffer variables

[Multipart message — attachments visible in raw view] — view raw

On Sun, Jul 17, 2016 at 08:15:11PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 17 Jul 2016 13:38:46 +0200
> 
> A few variables were assigned a null pointer despite of the detail
> that they were immediately reassigned by the following statement.
> Thus remove such unnecessary assignments.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Reviewed-by: Wolfram Sang <wsa@the-dreams.de>

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

#1445071 — [PATCH 1/9] staging: ks7010: Delete unnecessary checks before the function call "kfree"

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 17 Jul 2016 13:14:57 +0200

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/ks7010/ks7010_sdio.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index b7337fd..7da6c84 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -732,8 +732,7 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
 		goto error_out;
 	}
  error_out:
-	if (data_buf)
-		kfree(data_buf);
+	kfree(data_buf);
 	return rc;
 }
 
@@ -763,8 +762,7 @@ static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address,
 		goto error_out;
 	}
  error_out:
-	if (read_buf)
-		kfree(read_buf);
+	kfree(read_buf);
 	return rc;
 }
 
@@ -879,8 +877,7 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv,
 	release_firmware(fw_entry);
  error_out0:
 	sdio_release_host(card->func);
-	if (rom_buf)
-		kfree(rom_buf);
+	kfree(rom_buf);
 	return rc;
 }
 
@@ -1199,9 +1196,7 @@ static void ks7010_sdio_remove(struct sdio_func *func)
 		unregister_netdev(netdev);
 
 		trx_device_exit(priv);
-		if (priv->ks_wlan_hw.read_buf) {
-			kfree(priv->ks_wlan_hw.read_buf);
-		}
+		kfree(priv->ks_wlan_hw.read_buf);
 		free_netdev(priv->net_dev);
 		card->priv = NULL;
 	}
-- 
2.9.1

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

#1447313 — Re: [PATCH 1/9] staging: ks7010: Delete unnecessary checks before the function call "kfree"

[Multipart message — attachments visible in raw view] — view raw

On Sun, Jul 17, 2016 at 08:10:48PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 17 Jul 2016 13:14:57 +0200
> 
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the calls is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Wolfram Sang <wsa@the-dreams.de>

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

#1445072 — [PATCH 3/9] staging: ks7010: Return directly after a failed kmalloc()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 17 Jul 2016 15:55:02 +0200

Return directly after a memory allocation failed at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/ks7010/ks7010_sdio.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 3622fba..9b954cb 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -713,10 +713,8 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
 	unsigned char *data_buf;
 
 	data_buf = kmalloc(sizeof(u32), GFP_KERNEL);
-	if (!data_buf) {
-		rc = 1;
-		goto error_out;
-	}
+	if (!data_buf)
+		return 1;
 
 	memcpy(data_buf, &index, sizeof(index));
 	retval = ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index));
@@ -744,10 +742,9 @@ static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address,
 	unsigned char *read_buf;
 
 	read_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL);
-	if (!read_buf) {
-		rc = 1;
-		goto error_out;
-	}
+	if (!read_buf)
+		return 1;
+
 	retval = ks7010_sdio_read(priv, address, read_buf, size);
 	if (retval) {
 		rc = 2;
@@ -777,10 +774,8 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv,
 
 	/* buffer allocate */
 	rom_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL);
-	if (!rom_buf) {
-		rc = 3;
-		goto error_out0;
-	}
+	if (!rom_buf)
+		return 3;
 
 	sdio_claim_host(card->func);
 
-- 
2.9.1

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

#1445088 — Re: [PATCH 3/9] staging: ks7010: Return directly after a failed kmalloc()

On Sun, 17 Jul 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 17 Jul 2016 15:55:02 +0200
>
> Return directly after a memory allocation failed at the beginning.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/staging/ks7010/ks7010_sdio.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> index 3622fba..9b954cb 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.c
> +++ b/drivers/staging/ks7010/ks7010_sdio.c
> @@ -713,10 +713,8 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
>  	unsigned char *data_buf;
>
>  	data_buf = kmalloc(sizeof(u32), GFP_KERNEL);
> -	if (!data_buf) {
> -		rc = 1;
> -		goto error_out;
> -	}
> +	if (!data_buf)
> +		return 1;

One could rather wonder why the function has such strange error values...

julia

>
>  	memcpy(data_buf, &index, sizeof(index));
>  	retval = ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index));
> @@ -744,10 +742,9 @@ static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address,
>  	unsigned char *read_buf;
>
>  	read_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL);
> -	if (!read_buf) {
> -		rc = 1;
> -		goto error_out;
> -	}
> +	if (!read_buf)
> +		return 1;
> +
>  	retval = ks7010_sdio_read(priv, address, read_buf, size);
>  	if (retval) {
>  		rc = 2;
> @@ -777,10 +774,8 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv,
>
>  	/* buffer allocate */
>  	rom_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL);
> -	if (!rom_buf) {
> -		rc = 3;
> -		goto error_out0;
> -	}
> +	if (!rom_buf)
> +		return 3;
>
>  	sdio_claim_host(card->func);
>
> --
> 2.9.1
>
>

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

#1447312 — Re: [PATCH 3/9] staging: ks7010: Return directly after a failed kmalloc()

[Multipart message — attachments visible in raw view] — view raw

On Sun, Jul 17, 2016 at 08:58:14PM +0200, Julia Lawall wrote:
> 
> 
> On Sun, 17 Jul 2016, SF Markus Elfring wrote:
> 
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sun, 17 Jul 2016 15:55:02 +0200
> >
> > Return directly after a memory allocation failed at the beginning.
> >
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  drivers/staging/ks7010/ks7010_sdio.c | 19 +++++++------------
> >  1 file changed, 7 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> > index 3622fba..9b954cb 100644
> > --- a/drivers/staging/ks7010/ks7010_sdio.c
> > +++ b/drivers/staging/ks7010/ks7010_sdio.c
> > @@ -713,10 +713,8 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
> >  	unsigned char *data_buf;
> >
> >  	data_buf = kmalloc(sizeof(u32), GFP_KERNEL);
> > -	if (!data_buf) {
> > -		rc = 1;
> > -		goto error_out;
> > -	}
> > +	if (!data_buf)
> > +		return 1;
> 
> One could rather wonder why the function has such strange error values...

Agreed. Markus, can you check if we can use -ENOMEM in those places.

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

#1447398 — Re: [PATCH 3/9] staging: ks7010: Return directly after a failed kmalloc()

>>> @@ -713,10 +713,8 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
>>>  	unsigned char *data_buf;
>>>
>>>  	data_buf = kmalloc(sizeof(u32), GFP_KERNEL);
>>> -	if (!data_buf) {
>>> -		rc = 1;
>>> -		goto error_out;
>>> -	}
>>> +	if (!data_buf)
>>> +		return 1;
>>
>> One could rather wonder why the function has such strange error values...
> 
> Agreed. Markus, can you check if we can use -ENOMEM in those places.

I find that I do not know this software good enough at the moment
so that I could safely decide on the shown special error values.
I guess that further clarification might be needed for affected
implementation details.

Regards,
Markus

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

#1447636 — Re: [PATCH 3/9] staging: ks7010: Return directly after a failed kmalloc()

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jul 20, 2016 at 08:40:11PM +0200, SF Markus Elfring wrote:
> >>> @@ -713,10 +713,8 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
> >>>  	unsigned char *data_buf;
> >>>
> >>>  	data_buf = kmalloc(sizeof(u32), GFP_KERNEL);
> >>> -	if (!data_buf) {
> >>> -		rc = 1;
> >>> -		goto error_out;
> >>> -	}
> >>> +	if (!data_buf)
> >>> +		return 1;
> >>
> >> One could rather wonder why the function has such strange error values...
> > 
> > Agreed. Markus, can you check if we can use -ENOMEM in those places.
> 
> I find that I do not know this software good enough at the moment
> so that I could safely decide on the shown special error values.
> I guess that further clarification might be needed for affected
> implementation details.

That's OK, too.

Acked-by: Wolfram Sang <wsa@the-dreams.de>

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

#1448444 — Re: staging: ks7010: Return directly after a failed kmalloc()

>> I guess that further clarification might be needed for affected
>> implementation details.
> 
> That's OK, too.
> 
> Acked-by: Wolfram Sang <wsa@the-dreams.de>

Does this acknowledgement include also the acceptance for
the suggested change around calls of the functions "sdio_claim_host"
and "sdio_release_host" within the implementation of the
function "ks7010_upload_firmware"?
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/staging/ks7010/ks7010_sdio.c?id=13123042d0dbf7635f052efc2ae69fd9af624f1d#n771

Regards,
Markus

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

#1448449 — Re: staging: ks7010: Return directly after a failed kmalloc()

[Multipart message — attachments visible in raw view] — view raw

On Fri, Jul 22, 2016 at 09:36:53AM +0200, SF Markus Elfring wrote:
> >> I guess that further clarification might be needed for affected
> >> implementation details.
> > 
> > That's OK, too.
> > 
> > Acked-by: Wolfram Sang <wsa@the-dreams.de>
> 
> Does this acknowledgement include also the acceptance for
> the suggested change around calls of the functions "sdio_claim_host"
> and "sdio_release_host" within the implementation of the
> function "ks7010_upload_firmware"?

Likely. Send a patch and we will see.

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

#1445073 — [PATCH 7/9] staging: ks7010: Replace three printk() calls by pr_err()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 17 Jul 2016 19:12:27 +0200

Prefer usage of the macro "pr_err" over the interface "printk".
Fix a typo in an error message.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/ks7010/ks7010_sdio.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 1e072e3..4b15aec 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -987,11 +987,11 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	/* private memory allocate */
 	netdev = alloc_etherdev(sizeof(*priv));
 	if (netdev == NULL) {
-		printk(KERN_ERR "ks7010 : Unable to alloc new net device\n");
+		pr_err("ks7010: Unable to alloc new net device\n");
 		goto release_irq;
 	}
 	if (dev_alloc_name(netdev, "wlan%d") < 0) {
-		printk(KERN_ERR "ks7010 :  Couldn't get name!\n");
+		pr_err("ks7010: Couldn't get name!\n");
 		goto free_dev;
 	}
 
@@ -1031,8 +1031,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	/* Upload firmware */
 	ret = ks7010_upload_firmware(priv, card);	/* firmware load */
 	if (ret) {
-		printk(KERN_ERR
-		       "ks7010: firmware load failed !! retern code = %d\n",
+		pr_err("ks7010: firmware load failed! return code = %d\n",
 		       ret);
 		goto free_buf;
 	}
-- 
2.9.1

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

#1445112 — Re: [PATCH 7/9] staging: ks7010: Replace three printk() calls by pr_err()

On Sun, 2016-07-17 at 20:27 +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 17 Jul 2016 19:12:27 +0200
> 
> Prefer usage of the macro "pr_err" over the interface "printk".
> Fix a typo in an error message.

Please and and use pr_fmt

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

#1447321 — Re: [PATCH 7/9] staging: ks7010: Replace three printk() calls by pr_err()

[Multipart message — attachments visible in raw view] — view raw

On Sun, Jul 17, 2016 at 01:26:03PM -0700, Joe Perches wrote:
> On Sun, 2016-07-17 at 20:27 +0200, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sun, 17 Jul 2016 19:12:27 +0200
> > 
> > Prefer usage of the macro "pr_err" over the interface "printk".
> > Fix a typo in an error message.
> 
> Please and and use pr_fmt

Can't we use dev_* on the SDIO device?

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

#1445074 — [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 17 Jul 2016 18:15:23 +0200

Some return values can also be directly used for various condition checks.
Thus remove a local variable for intermediate assignments.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/ks7010/ks7010_sdio.c | 81 +++++++++++++++---------------------
 1 file changed, 34 insertions(+), 47 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index eea18fb..b3ca8e2 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -90,7 +90,6 @@ static int ks7010_sdio_write(struct ks_wlan_private *priv, unsigned int address,
 void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
 {
 	unsigned char rw_data;
-	int retval;
 
 	DPRINTK(4, "\n");
 
@@ -99,9 +98,10 @@ void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
 
 	if (atomic_read(&priv->sleepstatus.status) == 0) {
 		rw_data = GCR_B_DOZE;
-		retval =
-		    ks7010_sdio_write(priv, GCR_B, &rw_data, sizeof(rw_data));
-		if (retval) {
+		if (ks7010_sdio_write(priv,
+				      GCR_B,
+				      &rw_data,
+				      sizeof(rw_data))) {
 			DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
 			goto out;
 		}
@@ -121,7 +121,6 @@ out:
 void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
 {
 	unsigned char rw_data;
-	int retval;
 
 	DPRINTK(4, "\n");
 
@@ -130,9 +129,10 @@ void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
 
 	if (atomic_read(&priv->sleepstatus.status) == 1) {
 		rw_data = WAKEUP_REQ;
-		retval =
-		    ks7010_sdio_write(priv, WAKEUP, &rw_data, sizeof(rw_data));
-		if (retval) {
+		if (ks7010_sdio_write(priv,
+				      WAKEUP,
+				      &rw_data,
+				      sizeof(rw_data))) {
 			DPRINTK(1, " error : WAKEUP=%02X\n", rw_data);
 			goto out;
 		}
@@ -152,16 +152,15 @@ out:
 void ks_wlan_hw_wakeup_request(struct ks_wlan_private *priv)
 {
 	unsigned char rw_data;
-	int retval;
 
 	DPRINTK(4, "\n");
 	if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
 		rw_data = WAKEUP_REQ;
-		retval =
-		    ks7010_sdio_write(priv, WAKEUP, &rw_data, sizeof(rw_data));
-		if (retval) {
+		if (ks7010_sdio_write(priv,
+				      WAKEUP,
+				      &rw_data,
+				      sizeof(rw_data)))
 			DPRINTK(1, " error : WAKEUP=%02X\n", rw_data);
-		}
 		DPRINTK(4, "wake up : WAKEUP=%02X\n", rw_data);
 		priv->last_wakeup = jiffies;
 		++priv->wakeup_count;
@@ -175,7 +174,6 @@ int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 {
 	int rc = 0;
 	unsigned char rw_data;
-	int retval;
 
 	if (priv->reg.powermgt == POWMGT_ACTIVE_MODE)
 		return rc;
@@ -198,11 +196,11 @@ int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 				if (!atomic_read(&priv->psstatus.confirm_wait)
 				    && !atomic_read(&priv->psstatus.snooze_guard)
 				    && !cnt_txqbody(priv)) {
-					retval =
-					    ks7010_sdio_read(priv, INT_PENDING,
+					if (ks7010_sdio_read(priv,
+							     INT_PENDING,
 							     &rw_data,
-							     sizeof(rw_data));
-					if (retval) {
+							     sizeof(rw_data))
+							     ) {
 						DPRINTK(1,
 							" error : INT_PENDING=%02X\n",
 							rw_data);
@@ -212,12 +210,11 @@ int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 					}
 					if (!rw_data) {
 						rw_data = GCR_B_DOZE;
-						retval =
-						    ks7010_sdio_write(priv,
+						if (ks7010_sdio_write(priv,
 								      GCR_B,
 								      &rw_data,
-								      sizeof(rw_data));
-						if (retval) {
+								      sizeof(rw_data))
+								      ) {
 							DPRINTK(1,
 								" error : GCR_B=%02X\n",
 								rw_data);
@@ -413,7 +410,6 @@ static void rx_event_task(unsigned long dev)
 static void ks_wlan_hw_rx(void *dev, uint16_t size)
 {
 	struct ks_wlan_private *priv = (struct ks_wlan_private *)dev;
-	int retval;
 	struct rx_device_buffer *rx_buffer;
 	struct hostif_hdr *hdr;
 	unsigned char read_status;
@@ -429,12 +425,11 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
 	}
 	rx_buffer = &priv->rx_dev.rx_dev_buff[priv->rx_dev.qtail];
 
-	retval =
-	    ks7010_sdio_read(priv, DATA_WINDOW, &rx_buffer->data[0],
-			     hif_align_size(size));
-	if (retval) {
+	if (ks7010_sdio_read(priv,
+			     DATA_WINDOW,
+			     &rx_buffer->data[0],
+			     hif_align_size(size)))
 		goto error_out;
-	}
 
 	/* length check */
 	if (size > 2046 || size == 0) {
@@ -446,12 +441,11 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
 #endif
 		/* rx_status update */
 		read_status = READ_STATUS_IDLE;
-		retval =
-		    ks7010_sdio_write(priv, READ_STATUS, &read_status,
-				      sizeof(read_status));
-		if (retval) {
+		if (ks7010_sdio_write(priv,
+				      READ_STATUS,
+				      &read_status,
+				      sizeof(read_status)))
 			DPRINTK(1, " error : READ_STATUS=%02X\n", read_status);
-		}
 		goto error_out;
 	}
 
@@ -462,12 +456,11 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
 
 	/* read status update */
 	read_status = READ_STATUS_IDLE;
-	retval =
-	    ks7010_sdio_write(priv, READ_STATUS, &read_status,
-			      sizeof(read_status));
-	if (retval) {
+	if (ks7010_sdio_write(priv,
+			      READ_STATUS,
+			      &read_status,
+			      sizeof(read_status)))
 		DPRINTK(1, " error : READ_STATUS=%02X\n", read_status);
-	}
 	DPRINTK(4, "READ_STATUS=%02X\n", read_status);
 
 	if (atomic_read(&priv->psstatus.confirm_wait)) {
@@ -489,7 +482,6 @@ static void ks7010_rw_function(struct work_struct *work)
 	struct hw_info_t *hw;
 	struct ks_wlan_private *priv;
 	unsigned char rw_data;
-	int retval;
 
 	hw = container_of(work, struct hw_info_t, rw_wq.work);
 	priv = container_of(hw, struct ks_wlan_private, ks_wlan_hw);
@@ -538,9 +530,7 @@ static void ks7010_rw_function(struct work_struct *work)
 	}
 
 	/* read (WriteStatus/ReadDataSize FN1:00_0014) */
-	retval =
-	    ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data, sizeof(rw_data));
-	if (retval) {
+	if (ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data, sizeof(rw_data))) {
 		DPRINTK(1, " error : WSTATUS_RSIZE=%02X psstatus=%d\n", rw_data,
 			atomic_read(&priv->psstatus.status));
 		goto release_host;
@@ -708,7 +698,6 @@ static void trx_device_exit(struct ks_wlan_private *priv)
 static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
 {
 	int rc = 0;
-	int retval;
 	unsigned char *data_buf;
 
 	data_buf = kmalloc(sizeof(u32), GFP_KERNEL);
@@ -716,14 +705,12 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
 		return 1;
 
 	memcpy(data_buf, &index, sizeof(index));
-	retval = ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index));
-	if (retval) {
+	if (ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index))) {
 		rc = 2;
 		goto free_buf;
 	}
 
-	retval = ks7010_sdio_write(priv, READ_INDEX, data_buf, sizeof(index));
-	if (retval)
+	if (ks7010_sdio_write(priv, READ_INDEX, data_buf, sizeof(index)))
 		rc = 3;
 free_buf:
 	kfree(data_buf);
-- 
2.9.1

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

#1445089 — Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

On Sun, 17 Jul 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 17 Jul 2016 18:15:23 +0200
>
> Some return values can also be directly used for various condition checks.
> Thus remove a local variable for intermediate assignments.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/staging/ks7010/ks7010_sdio.c | 81 +++++++++++++++---------------------
>  1 file changed, 34 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> index eea18fb..b3ca8e2 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.c
> +++ b/drivers/staging/ks7010/ks7010_sdio.c
> @@ -90,7 +90,6 @@ static int ks7010_sdio_write(struct ks_wlan_private *priv, unsigned int address,
>  void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
>  {
>  	unsigned char rw_data;
> -	int retval;
>
>  	DPRINTK(4, "\n");
>
> @@ -99,9 +98,10 @@ void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
>
>  	if (atomic_read(&priv->sleepstatus.status) == 0) {
>  		rw_data = GCR_B_DOZE;
> -		retval =
> -		    ks7010_sdio_write(priv, GCR_B, &rw_data, sizeof(rw_data));
> -		if (retval) {
> +		if (ks7010_sdio_write(priv,
> +				      GCR_B,
> +				      &rw_data,
> +				      sizeof(rw_data))) {

A multi-line function call in an if test does not look nice at all.  The
original code was an easy-to-read expectable pattern.

julia

>  			DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
>  			goto out;
>  		}
> @@ -121,7 +121,6 @@ out:
>  void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
>  {
>  	unsigned char rw_data;
> -	int retval;
>
>  	DPRINTK(4, "\n");
>
> @@ -130,9 +129,10 @@ void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
>
>  	if (atomic_read(&priv->sleepstatus.status) == 1) {
>  		rw_data = WAKEUP_REQ;
> -		retval =
> -		    ks7010_sdio_write(priv, WAKEUP, &rw_data, sizeof(rw_data));
> -		if (retval) {
> +		if (ks7010_sdio_write(priv,
> +				      WAKEUP,
> +				      &rw_data,
> +				      sizeof(rw_data))) {
>  			DPRINTK(1, " error : WAKEUP=%02X\n", rw_data);
>  			goto out;
>  		}
> @@ -152,16 +152,15 @@ out:
>  void ks_wlan_hw_wakeup_request(struct ks_wlan_private *priv)
>  {
>  	unsigned char rw_data;
> -	int retval;
>
>  	DPRINTK(4, "\n");
>  	if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
>  		rw_data = WAKEUP_REQ;
> -		retval =
> -		    ks7010_sdio_write(priv, WAKEUP, &rw_data, sizeof(rw_data));
> -		if (retval) {
> +		if (ks7010_sdio_write(priv,
> +				      WAKEUP,
> +				      &rw_data,
> +				      sizeof(rw_data)))
>  			DPRINTK(1, " error : WAKEUP=%02X\n", rw_data);
> -		}
>  		DPRINTK(4, "wake up : WAKEUP=%02X\n", rw_data);
>  		priv->last_wakeup = jiffies;
>  		++priv->wakeup_count;
> @@ -175,7 +174,6 @@ int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
>  {
>  	int rc = 0;
>  	unsigned char rw_data;
> -	int retval;
>
>  	if (priv->reg.powermgt == POWMGT_ACTIVE_MODE)
>  		return rc;
> @@ -198,11 +196,11 @@ int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
>  				if (!atomic_read(&priv->psstatus.confirm_wait)
>  				    && !atomic_read(&priv->psstatus.snooze_guard)
>  				    && !cnt_txqbody(priv)) {
> -					retval =
> -					    ks7010_sdio_read(priv, INT_PENDING,
> +					if (ks7010_sdio_read(priv,
> +							     INT_PENDING,
>  							     &rw_data,
> -							     sizeof(rw_data));
> -					if (retval) {
> +							     sizeof(rw_data))
> +							     ) {
>  						DPRINTK(1,
>  							" error : INT_PENDING=%02X\n",
>  							rw_data);
> @@ -212,12 +210,11 @@ int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
>  					}
>  					if (!rw_data) {
>  						rw_data = GCR_B_DOZE;
> -						retval =
> -						    ks7010_sdio_write(priv,
> +						if (ks7010_sdio_write(priv,
>  								      GCR_B,
>  								      &rw_data,
> -								      sizeof(rw_data));
> -						if (retval) {
> +								      sizeof(rw_data))
> +								      ) {
>  							DPRINTK(1,
>  								" error : GCR_B=%02X\n",
>  								rw_data);
> @@ -413,7 +410,6 @@ static void rx_event_task(unsigned long dev)
>  static void ks_wlan_hw_rx(void *dev, uint16_t size)
>  {
>  	struct ks_wlan_private *priv = (struct ks_wlan_private *)dev;
> -	int retval;
>  	struct rx_device_buffer *rx_buffer;
>  	struct hostif_hdr *hdr;
>  	unsigned char read_status;
> @@ -429,12 +425,11 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
>  	}
>  	rx_buffer = &priv->rx_dev.rx_dev_buff[priv->rx_dev.qtail];
>
> -	retval =
> -	    ks7010_sdio_read(priv, DATA_WINDOW, &rx_buffer->data[0],
> -			     hif_align_size(size));
> -	if (retval) {
> +	if (ks7010_sdio_read(priv,
> +			     DATA_WINDOW,
> +			     &rx_buffer->data[0],
> +			     hif_align_size(size)))
>  		goto error_out;
> -	}
>
>  	/* length check */
>  	if (size > 2046 || size == 0) {
> @@ -446,12 +441,11 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
>  #endif
>  		/* rx_status update */
>  		read_status = READ_STATUS_IDLE;
> -		retval =
> -		    ks7010_sdio_write(priv, READ_STATUS, &read_status,
> -				      sizeof(read_status));
> -		if (retval) {
> +		if (ks7010_sdio_write(priv,
> +				      READ_STATUS,
> +				      &read_status,
> +				      sizeof(read_status)))
>  			DPRINTK(1, " error : READ_STATUS=%02X\n", read_status);
> -		}
>  		goto error_out;
>  	}
>
> @@ -462,12 +456,11 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
>
>  	/* read status update */
>  	read_status = READ_STATUS_IDLE;
> -	retval =
> -	    ks7010_sdio_write(priv, READ_STATUS, &read_status,
> -			      sizeof(read_status));
> -	if (retval) {
> +	if (ks7010_sdio_write(priv,
> +			      READ_STATUS,
> +			      &read_status,
> +			      sizeof(read_status)))
>  		DPRINTK(1, " error : READ_STATUS=%02X\n", read_status);
> -	}
>  	DPRINTK(4, "READ_STATUS=%02X\n", read_status);
>
>  	if (atomic_read(&priv->psstatus.confirm_wait)) {
> @@ -489,7 +482,6 @@ static void ks7010_rw_function(struct work_struct *work)
>  	struct hw_info_t *hw;
>  	struct ks_wlan_private *priv;
>  	unsigned char rw_data;
> -	int retval;
>
>  	hw = container_of(work, struct hw_info_t, rw_wq.work);
>  	priv = container_of(hw, struct ks_wlan_private, ks_wlan_hw);
> @@ -538,9 +530,7 @@ static void ks7010_rw_function(struct work_struct *work)
>  	}
>
>  	/* read (WriteStatus/ReadDataSize FN1:00_0014) */
> -	retval =
> -	    ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data, sizeof(rw_data));
> -	if (retval) {
> +	if (ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data, sizeof(rw_data))) {
>  		DPRINTK(1, " error : WSTATUS_RSIZE=%02X psstatus=%d\n", rw_data,
>  			atomic_read(&priv->psstatus.status));
>  		goto release_host;
> @@ -708,7 +698,6 @@ static void trx_device_exit(struct ks_wlan_private *priv)
>  static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
>  {
>  	int rc = 0;
> -	int retval;
>  	unsigned char *data_buf;
>
>  	data_buf = kmalloc(sizeof(u32), GFP_KERNEL);
> @@ -716,14 +705,12 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
>  		return 1;
>
>  	memcpy(data_buf, &index, sizeof(index));
> -	retval = ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index));
> -	if (retval) {
> +	if (ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index))) {
>  		rc = 2;
>  		goto free_buf;
>  	}
>
> -	retval = ks7010_sdio_write(priv, READ_INDEX, data_buf, sizeof(index));
> -	if (retval)
> +	if (ks7010_sdio_write(priv, READ_INDEX, data_buf, sizeof(index)))
>  		rc = 3;
>  free_buf:
>  	kfree(data_buf);
> --
> 2.9.1
>
>

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

#1447317 — Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

[Multipart message — attachments visible in raw view] — view raw

On Sun, Jul 17, 2016 at 08:56:59PM +0200, Julia Lawall wrote:
> 
> 
> On Sun, 17 Jul 2016, SF Markus Elfring wrote:
> 
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sun, 17 Jul 2016 18:15:23 +0200
> >
> > Some return values can also be directly used for various condition checks.
> > Thus remove a local variable for intermediate assignments.
> >
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  drivers/staging/ks7010/ks7010_sdio.c | 81 +++++++++++++++---------------------
> >  1 file changed, 34 insertions(+), 47 deletions(-)
> >
> > diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> > index eea18fb..b3ca8e2 100644
> > --- a/drivers/staging/ks7010/ks7010_sdio.c
> > +++ b/drivers/staging/ks7010/ks7010_sdio.c
> > @@ -90,7 +90,6 @@ static int ks7010_sdio_write(struct ks_wlan_private *priv, unsigned int address,
> >  void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
> >  {
> >  	unsigned char rw_data;
> > -	int retval;
> >
> >  	DPRINTK(4, "\n");
> >
> > @@ -99,9 +98,10 @@ void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
> >
> >  	if (atomic_read(&priv->sleepstatus.status) == 0) {
> >  		rw_data = GCR_B_DOZE;
> > -		retval =
> > -		    ks7010_sdio_write(priv, GCR_B, &rw_data, sizeof(rw_data));
> > -		if (retval) {
> > +		if (ks7010_sdio_write(priv,
> > +				      GCR_B,
> > +				      &rw_data,
> > +				      sizeof(rw_data))) {
> 
> A multi-line function call in an if test does not look nice at all.  The
> original code was an easy-to-read expectable pattern.

I agree. I am not strict on the 80 char limit, especially in cases like
the above.

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

#1447396 — Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

>>> @@ -90,7 +90,6 @@ static int ks7010_sdio_write(struct ks_wlan_private *priv, unsigned int address,
>>>  void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
>>>  {
>>>  	unsigned char rw_data;
>>> -	int retval;
>>>
>>>  	DPRINTK(4, "\n");
>>>
>>> @@ -99,9 +98,10 @@ void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
>>>
>>>  	if (atomic_read(&priv->sleepstatus.status) == 0) {
>>>  		rw_data = GCR_B_DOZE;
>>> -		retval =
>>> -		    ks7010_sdio_write(priv, GCR_B, &rw_data, sizeof(rw_data));
>>> -		if (retval) {
>>> +		if (ks7010_sdio_write(priv,
>>> +				      GCR_B,
>>> +				      &rw_data,
>>> +				      sizeof(rw_data))) {
>>
>> A multi-line function call in an if test does not look nice at all.  The
>> original code was an easy-to-read expectable pattern.
> 
> I agree. I am not strict on the 80 char limit, especially in cases like
> the above.

Would you try an other source code formatting for the suggested change pattern?

Regards,
Markus

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

#1447635 — Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

[Multipart message — attachments visible in raw view] — view raw

> >>>  	if (atomic_read(&priv->sleepstatus.status) == 0) {
> >>>  		rw_data = GCR_B_DOZE;
> >>> -		retval =
> >>> -		    ks7010_sdio_write(priv, GCR_B, &rw_data, sizeof(rw_data));
> >>> -		if (retval) {
> >>> +		if (ks7010_sdio_write(priv,
> >>> +				      GCR_B,
> >>> +				      &rw_data,
> >>> +				      sizeof(rw_data))) {
> >>
> >> A multi-line function call in an if test does not look nice at all.  The
> >> original code was an easy-to-read expectable pattern.
> > 
> > I agree. I am not strict on the 80 char limit, especially in cases like
> > the above.
> 
> Would you try an other source code formatting for the suggested change pattern?

I don't understand the question?

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

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web