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

Groups > linux.kernel > #1299983 > unrolled thread

[PATCH 0/3] net-iwlegacy: Fine-tuning for il_eeprom_init()

Started bySF Markus Elfring <elfring@users.sourceforge.net>
First post2016-01-01 21:30 +0100
Last post2016-01-02 19:20 +0100
Articles 7 — 4 participants

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/3] net-iwlegacy: Fine-tuning for il_eeprom_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-01 21:30 +0100
    [PATCH 2/3] net-iwlegacy: One check less in il_eeprom_init() after  error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-01 21:40 +0100
      Re: [PATCH 2/3] net-iwlegacy: One check less in il_eeprom_init()  after error detection Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-01-02 00:20 +0100
    [PATCH 1/3] net-iwlegacy: Refactoring for il_eeprom_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-01 21:40 +0100
      Re: [PATCH 1/3] net-iwlegacy: Refactoring for il_eeprom_init() Stanislaw Gruszka <sgruszka@redhat.com> - 2016-01-04 10:40 +0100
    [PATCH 3/3] net-iwlegacy: Another refactoring for il_eeprom_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-01 21:40 +0100
      Re: [PATCH 3/3] net-iwlegacy: Another refactoring for il_eeprom_init() Souptick Joarder <jrdr.linux@gmail.com> - 2016-01-02 19:20 +0100

#1299983 — [PATCH 0/3] net-iwlegacy: Fine-tuning for il_eeprom_init()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-01-01 21:30 +0100
Subject[PATCH 0/3] net-iwlegacy: Fine-tuning for il_eeprom_init()
Message-ID<qMeuJ-780-3@gated-at.bofh.it>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 1 Jan 2016 21:25:43 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Refactoring
  One check less after error detection
  Another refactoring

 drivers/net/wireless/intel/iwlegacy/common.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

-- 
2.6.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]

#1299984 — [PATCH 2/3] net-iwlegacy: One check less in il_eeprom_init() after error detection

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-01-01 21:40 +0100
Subject[PATCH 2/3] net-iwlegacy: One check less in il_eeprom_init() after error detection
Message-ID<qMeEp-7bc-7@gated-at.bofh.it>
In reply to#1299983
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 1 Jan 2016 21:12:29 +0100

This issue was detected by using the Coccinelle software.

Adjust a jump target to avoid a check repetition before the function
call "il_eeprom_free".

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/intel/iwlegacy/common.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c
index c3afaf7..ae45fd3 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.c
+++ b/drivers/net/wireless/intel/iwlegacy/common.c
@@ -734,7 +734,7 @@ il_eeprom_init(struct il_priv *il)
 	if (ret < 0) {
 		IL_ERR("EEPROM not found, EEPROM_GP=0x%08x\n", gp);
 		ret = -ENOENT;
-		goto err;
+		goto free_eeprom;
 	}
 
 	/* Make sure driver (instead of uCode) is allowed to read EEPROM */
@@ -742,7 +742,7 @@ il_eeprom_init(struct il_priv *il)
 	if (ret < 0) {
 		IL_ERR("Failed to acquire EEPROM semaphore.\n");
 		ret = -ENOENT;
-		goto err;
+		goto free_eeprom;
 	}
 
 	/* eeprom is an array of 16bit values */
@@ -772,9 +772,11 @@ il_eeprom_init(struct il_priv *il)
 done:
 	il->ops->eeprom_release_semaphore(il);
 
-err:
-	if (ret)
+	if (ret) {
+free_eeprom:
 		il_eeprom_free(il);
+	}
+
 	/* Reset chip to save power until we load uCode during "up". */
 	il_apm_stop(il);
 	return ret;
-- 
2.6.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] | [prev] | [next] | [standalone]

#1300009 — Re: [PATCH 2/3] net-iwlegacy: One check less in il_eeprom_init() after error detection

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2016-01-02 00:20 +0100
SubjectRe: [PATCH 2/3] net-iwlegacy: One check less in il_eeprom_init() after error detection
Message-ID<qMh9f-ms-5@gated-at.bofh.it>
In reply to#1299984
Hello.

On 1/1/2016 11:31 PM, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 1 Jan 2016 21:12:29 +0100
>
> This issue was detected by using the Coccinelle software.
>
> Adjust a jump target to avoid a check repetition before the function
> call "il_eeprom_free".

    One question: why?

>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>   drivers/net/wireless/intel/iwlegacy/common.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c
> index c3afaf7..ae45fd3 100644
> --- a/drivers/net/wireless/intel/iwlegacy/common.c
> +++ b/drivers/net/wireless/intel/iwlegacy/common.c
[...]
> @@ -772,9 +772,11 @@ il_eeprom_init(struct il_priv *il)
>   done:
>   	il->ops->eeprom_release_semaphore(il);
>
> -err:
> -	if (ret)
> +	if (ret) {
> +free_eeprom:

    This is ugly, I'd say.

[...]

MBR, Sergei

--
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]

#1299985 — [PATCH 1/3] net-iwlegacy: Refactoring for il_eeprom_init()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-01-01 21:40 +0100
Subject[PATCH 1/3] net-iwlegacy: Refactoring for il_eeprom_init()
Message-ID<qMeEq-7bc-13@gated-at.bofh.it>
In reply to#1299983
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 1 Jan 2016 20:54:25 +0100

Return directly if a memory allocation failed at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/intel/iwlegacy/common.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c
index eb5cb60..c3afaf7 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.c
+++ b/drivers/net/wireless/intel/iwlegacy/common.c
@@ -723,10 +723,9 @@ il_eeprom_init(struct il_priv *il)
 	sz = il->cfg->eeprom_size;
 	D_EEPROM("NVM size = %d\n", sz);
 	il->eeprom = kzalloc(sz, GFP_KERNEL);
-	if (!il->eeprom) {
-		ret = -ENOMEM;
-		goto alloc_err;
-	}
+	if (!il->eeprom)
+		return -ENOMEM;
+
 	e = (__le16 *) il->eeprom;
 
 	il->ops->apm_init(il);
@@ -778,7 +777,6 @@ err:
 		il_eeprom_free(il);
 	/* Reset chip to save power until we load uCode during "up". */
 	il_apm_stop(il);
-alloc_err:
 	return ret;
 }
 EXPORT_SYMBOL(il_eeprom_init);
-- 
2.6.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] | [prev] | [next] | [standalone]

#1300575 — Re: [PATCH 1/3] net-iwlegacy: Refactoring for il_eeprom_init()

FromStanislaw Gruszka <sgruszka@redhat.com>
Date2016-01-04 10:40 +0100
SubjectRe: [PATCH 1/3] net-iwlegacy: Refactoring for il_eeprom_init()
Message-ID<qN9Mm-1Eg-3@gated-at.bofh.it>
In reply to#1299985
On Fri, Jan 01, 2016 at 09:30:10PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 1 Jan 2016 20:54:25 +0100
> 
> Return directly if a memory allocation failed at the beginning.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>

--
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]

#1299986 — [PATCH 3/3] net-iwlegacy: Another refactoring for il_eeprom_init()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-01-01 21:40 +0100
Subject[PATCH 3/3] net-iwlegacy: Another refactoring for il_eeprom_init()
Message-ID<qMeEq-7bc-11@gated-at.bofh.it>
In reply to#1299983
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 1 Jan 2016 21:16:01 +0100

Rename a jump label according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/intel/iwlegacy/common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c
index ae45fd3..660ab2b 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.c
+++ b/drivers/net/wireless/intel/iwlegacy/common.c
@@ -759,7 +759,7 @@ il_eeprom_init(struct il_priv *il)
 				 IL_EEPROM_ACCESS_TIMEOUT);
 		if (ret < 0) {
 			IL_ERR("Time out reading EEPROM[%d]\n", addr);
-			goto done;
+			goto release_semaphore;
 		}
 		r = _il_rd(il, CSR_EEPROM_REG);
 		e[addr / 2] = cpu_to_le16(r >> 16);
@@ -769,7 +769,7 @@ il_eeprom_init(struct il_priv *il)
 		 il_eeprom_query16(il, EEPROM_VERSION));
 
 	ret = 0;
-done:
+release_semaphore:
 	il->ops->eeprom_release_semaphore(il);
 
 	if (ret) {
-- 
2.6.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] | [prev] | [next] | [standalone]

#1300106 — Re: [PATCH 3/3] net-iwlegacy: Another refactoring for il_eeprom_init()

FromSouptick Joarder <jrdr.linux@gmail.com>
Date2016-01-02 19:20 +0100
SubjectRe: [PATCH 3/3] net-iwlegacy: Another refactoring for il_eeprom_init()
Message-ID<qMyWu-34r-5@gated-at.bofh.it>
In reply to#1299986
On Sat, Jan 2, 2016 at 2:02 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 1 Jan 2016 21:16:01 +0100
>
> Rename a jump label according to the current Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/net/wireless/intel/iwlegacy/common.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c
> index ae45fd3..660ab2b 100644
> --- a/drivers/net/wireless/intel/iwlegacy/common.c
> +++ b/drivers/net/wireless/intel/iwlegacy/common.c
> @@ -759,7 +759,7 @@ il_eeprom_init(struct il_priv *il)
>                                  IL_EEPROM_ACCESS_TIMEOUT);
>                 if (ret < 0) {
>                         IL_ERR("Time out reading EEPROM[%d]\n", addr);
> -                       goto done;
> +                       goto release_semaphore;

Current code looks good.
>                 }
>                 r = _il_rd(il, CSR_EEPROM_REG);
>                 e[addr / 2] = cpu_to_le16(r >> 16);
> @@ -769,7 +769,7 @@ il_eeprom_init(struct il_priv *il)
>                  il_eeprom_query16(il, EEPROM_VERSION));
>
>         ret = 0;
> -done:
> +release_semaphore:
>         il->ops->eeprom_release_semaphore(il);
>
>         if (ret) {
> --
> 2.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-Souptick
--
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