2023-05-09 19:39:44

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH 2/2] mtd: spi-nor: spansion: make sure local struct does not contain garbage

Following error was seen with um-x86_64-gcc12/um-allyesconfig:
+ /kisskb/src/drivers/mtd/spi-nor/spansion.c: error: 'op' is used uninitialized [-Werror=uninitialized]: => 495:27, 364:27

Set the spi_mem_op fields at the time of declaration so that all
uninitialized members be initialized to default value.

Reported-by: Geert Uytterhoeven <[email protected]>
Fixes: 6afcc84080c41 ("mtd: spi-nor: spansion: Add support for Infineon S25FS256T")
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spansion.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index d44ffaac1dba..257d05ed4146 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -492,7 +492,10 @@ s25fs256t_post_bfpt_fixup(struct spi_nor *nor,
const struct sfdp_parameter_header *bfpt_header,
const struct sfdp_bfpt *bfpt)
{
- struct spi_mem_op op;
+ struct spi_mem_op op =
+ CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes,
+ SPINOR_REG_CYPRESS_ARCFN, 1,
+ nor->bouncebuf);
int ret;

ret = cypress_nor_set_addr_mode_nbytes(nor);
@@ -500,10 +503,6 @@ s25fs256t_post_bfpt_fixup(struct spi_nor *nor,
return ret;

/* Read Architecture Configuration Register (ARCFN) */
- op = (struct spi_mem_op)
- CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes,
- SPINOR_REG_CYPRESS_ARCFN, 1,
- nor->bouncebuf);
ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
if (ret)
return ret;
--
2.40.1.521.gf1e218fcd8-goog


2023-05-09 19:57:38

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH 2/2] mtd: spi-nor: spansion: make sure local struct does not contain garbage



On 5/9/23 20:25, Tudor Ambarus wrote:
> Following error was seen with um-x86_64-gcc12/um-allyesconfig:
> + /kisskb/src/drivers/mtd/spi-nor/spansion.c: error: 'op' is used uninitialized [-Werror=uninitialized]: => 495:27, 364:27
>
> Set the spi_mem_op fields at the time of declaration so that all
> uninitialized members be initialized to default value.
>
> Reported-by: Geert Uytterhoeven <[email protected]>
> Fixes: 6afcc84080c41 ("mtd: spi-nor: spansion: Add support for Infineon S25FS256T")
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
> drivers/mtd/spi-nor/spansion.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index d44ffaac1dba..257d05ed4146 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -492,7 +492,10 @@ s25fs256t_post_bfpt_fixup(struct spi_nor *nor,
> const struct sfdp_parameter_header *bfpt_header,
> const struct sfdp_bfpt *bfpt)
> {
> - struct spi_mem_op op;
> + struct spi_mem_op op =
> + CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes,
> + SPINOR_REG_CYPRESS_ARCFN, 1,
> + nor->bouncebuf);
> int ret;
>

oops, this is wrong as addr_mode_nbytes is updated just below in
cypress_nor_set_addr_mode_nbytes(), so the fix is to init op with {} at
declaration, as in the previous patch.

> ret = cypress_nor_set_addr_mode_nbytes(nor);
> @@ -500,10 +503,6 @@ s25fs256t_post_bfpt_fixup(struct spi_nor *nor,
> return ret;
>
> /* Read Architecture Configuration Register (ARCFN) */
> - op = (struct spi_mem_op)
> - CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes,
> - SPINOR_REG_CYPRESS_ARCFN, 1,
> - nor->bouncebuf);
> ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
> if (ret)
> return ret;