diff mbox

[v2] mmc: sdhci-cadence: fix logically and structurally dead code

Message ID 20180419155958.GA18593@embeddedor.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gustavo A. R. Silva April 19, 2018, 3:59 p.m. UTC
Currently, the code block inside the for loop will never execute
more than once, because the function returns inmediately after
the first iteration, hence the execution of the code at the second
iteration is structurally dead and, code at line 281: return 0; is
never reached.

Fix this by checking _ret_ before return.

Addresses-Coverity-ID: 1468009 ("Logically dead code")
Addresses-Coverity-ID: 1468002 ("Structurally dead code")
Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
Changes in v2:
 - Update changelog.
 - Drop the 'Fixes' tag.
 - Add check on ret instead of removing the "return ret;" line.
 - Thanks to Masahiro Yamada for the feedback provided.

 drivers/mmc/host/sdhci-cadence.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Masahiro Yamada April 20, 2018, 1:07 a.m. UTC | #1
2018-04-20 0:59 GMT+09:00 Gustavo A. R. Silva <gustavo@embeddedor.com>:
> Currently, the code block inside the for loop will never execute
> more than once, because the function returns inmediately after
> the first iteration, hence the execution of the code at the second
> iteration is structurally dead and, code at line 281: return 0; is
> never reached.
>
> Fix this by checking _ret_ before return.
>
> Addresses-Coverity-ID: 1468009 ("Logically dead code")
> Addresses-Coverity-ID: 1468002 ("Structurally dead code")
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---

Looks good to me except the typo in the log

inmediately -> immediately


Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>


> Changes in v2:
>  - Update changelog.
>  - Drop the 'Fixes' tag.
>  - Add check on ret instead of removing the "return ret;" line.
>  - Thanks to Masahiro Yamada for the feedback provided.
>
>  drivers/mmc/host/sdhci-cadence.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index bc30d16..7a343b8 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -274,8 +274,8 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
>                 ret = readl_poll_timeout(reg, tmp,
>                                          !(tmp & SDHCI_CDNS_HRS06_TUNE_UP),
>                                          0, 1);
> -
> -               return ret;
> +               if (ret)
> +                       return ret;
>         }
>
>         return 0;
> --
> 2.7.4
>
Adrian Hunter April 20, 2018, 6:08 a.m. UTC | #2
On 19/04/18 18:59, Gustavo A. R. Silva wrote:
> Currently, the code block inside the for loop will never execute
> more than once, because the function returns inmediately after
> the first iteration, hence the execution of the code at the second
> iteration is structurally dead and, code at line 281: return 0; is
> never reached.
> 
> Fix this by checking _ret_ before return.
> 
> Addresses-Coverity-ID: 1468009 ("Logically dead code")
> Addresses-Coverity-ID: 1468002 ("Structurally dead code")
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> Changes in v2:
>  - Update changelog.
>  - Drop the 'Fixes' tag.
>  - Add check on ret instead of removing the "return ret;" line.
>  - Thanks to Masahiro Yamada for the feedback provided.
> 
>  drivers/mmc/host/sdhci-cadence.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index bc30d16..7a343b8 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -274,8 +274,8 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
>  		ret = readl_poll_timeout(reg, tmp,
>  					 !(tmp & SDHCI_CDNS_HRS06_TUNE_UP),
>  					 0, 1);
> -
> -		return ret;
> +		if (ret)
> +			return ret;
>  	}
>  
>  	return 0;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson April 20, 2018, 7:10 a.m. UTC | #3
On 19 April 2018 at 17:59, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote:
> Currently, the code block inside the for loop will never execute
> more than once, because the function returns inmediately after
> the first iteration, hence the execution of the code at the second
> iteration is structurally dead and, code at line 281: return 0; is
> never reached.
>
> Fix this by checking _ret_ before return.
>
> Addresses-Coverity-ID: 1468009 ("Logically dead code")
> Addresses-Coverity-ID: 1468002 ("Structurally dead code")
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Thanks, applied for next and by fixing the minor spelling mistake in
the changelog.

Kind regards
Uffe

> ---
> Changes in v2:
>  - Update changelog.
>  - Drop the 'Fixes' tag.
>  - Add check on ret instead of removing the "return ret;" line.
>  - Thanks to Masahiro Yamada for the feedback provided.
>
>  drivers/mmc/host/sdhci-cadence.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index bc30d16..7a343b8 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -274,8 +274,8 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
>                 ret = readl_poll_timeout(reg, tmp,
>                                          !(tmp & SDHCI_CDNS_HRS06_TUNE_UP),
>                                          0, 1);
> -
> -               return ret;
> +               if (ret)
> +                       return ret;
>         }
>
>         return 0;
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
index bc30d16..7a343b8 100644
--- a/drivers/mmc/host/sdhci-cadence.c
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -274,8 +274,8 @@  static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
 		ret = readl_poll_timeout(reg, tmp,
 					 !(tmp & SDHCI_CDNS_HRS06_TUNE_UP),
 					 0, 1);
-
-		return ret;
+		if (ret)
+			return ret;
 	}
 
 	return 0;