* [PATCH] mtd: spi-nor: stm32-quadspi: avoid unintialized return code @ 2017-09-14 11:06 Arnd Bergmann 2017-09-14 12:38 ` Ludovic BARRE 2017-09-14 13:38 ` Geert Uytterhoeven 0 siblings, 2 replies; 8+ messages in thread From: Arnd Bergmann @ 2017-09-14 11:06 UTC (permalink / raw) To: Cyrille Pitchen, Marek Vasut Cc: Arnd Bergmann, David Woodhouse, Brian Norris, Boris Brezillon, Richard Weinberger, Maxime Coquelin, Alexandre Torgue, Ludovic Barre, linux-mtd, linux-arm-kernel, linux-kernel If we send zero-length data to stm32_qspi_tx_poll() on older compiler versions such as gcc-4.6, we get warned that the return code is uninitialized: drivers/mtd/spi-nor/stm32-quadspi.c:248:2: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized] On newer compiler versions, the return code is always zero in this case, as the local variable gets optimized away and is assumed to be zero after the loop completes without error. This changes the function to instead return -EINVAL if it ever gets called with a zero length buffer. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82203 Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/mtd/spi-nor/stm32-quadspi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c index 86c0931543c5..711cfe7aa4bf 100644 --- a/drivers/mtd/spi-nor/stm32-quadspi.c +++ b/drivers/mtd/spi-nor/stm32-quadspi.c @@ -227,7 +227,7 @@ static int stm32_qspi_tx_poll(struct stm32_qspi *qspi, void (*tx_fifo)(u8 *, void __iomem *); u32 len = cmd->len, sr; u8 *buf = cmd->buf; - int ret; + int ret = -EINVAL; if (cmd->qspimode == CCR_FMODE_INDW) tx_fifo = stm32_qspi_write_fifo; -- 2.9.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: stm32-quadspi: avoid unintialized return code 2017-09-14 11:06 [PATCH] mtd: spi-nor: stm32-quadspi: avoid unintialized return code Arnd Bergmann @ 2017-09-14 12:38 ` Ludovic BARRE 2017-09-14 13:38 ` Geert Uytterhoeven 1 sibling, 0 replies; 8+ messages in thread From: Ludovic BARRE @ 2017-09-14 12:38 UTC (permalink / raw) To: Arnd Bergmann, Cyrille Pitchen, Marek Vasut Cc: David Woodhouse, Brian Norris, Boris Brezillon, Richard Weinberger, Maxime Coquelin, Alexandre Torgue, linux-mtd, linux-arm-kernel, linux-kernel hi Arnd thx Arnd for compilation warning (gcc <= 4.6) Acked-by: Ludovic Barre <ludovic.barre@st.com> PS: at runtime stm32_qspi_tx_poll can't be call, because stm32_qspi_tx check if there is tx data available if (!cmd->tx_data) return 0; BR Ludo On 09/14/2017 01:06 PM, Arnd Bergmann wrote: > If we send zero-length data to stm32_qspi_tx_poll() on older > compiler versions such as gcc-4.6, we get warned that the > return code is uninitialized: > > drivers/mtd/spi-nor/stm32-quadspi.c:248:2: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized] > > On newer compiler versions, the return code is always zero > in this case, as the local variable gets optimized away and > is assumed to be zero after the loop completes without error. > > This changes the function to instead return -EINVAL if it > ever gets called with a zero length buffer. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82203 > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/mtd/spi-nor/stm32-quadspi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c > index 86c0931543c5..711cfe7aa4bf 100644 > --- a/drivers/mtd/spi-nor/stm32-quadspi.c > +++ b/drivers/mtd/spi-nor/stm32-quadspi.c > @@ -227,7 +227,7 @@ static int stm32_qspi_tx_poll(struct stm32_qspi *qspi, > void (*tx_fifo)(u8 *, void __iomem *); > u32 len = cmd->len, sr; > u8 *buf = cmd->buf; > - int ret; > + int ret = -EINVAL; > > if (cmd->qspimode == CCR_FMODE_INDW) > tx_fifo = stm32_qspi_write_fifo; > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: stm32-quadspi: avoid unintialized return code 2017-09-14 11:06 [PATCH] mtd: spi-nor: stm32-quadspi: avoid unintialized return code Arnd Bergmann 2017-09-14 12:38 ` Ludovic BARRE @ 2017-09-14 13:38 ` Geert Uytterhoeven 2017-09-14 15:13 ` Ludovic BARRE 1 sibling, 1 reply; 8+ messages in thread From: Geert Uytterhoeven @ 2017-09-14 13:38 UTC (permalink / raw) To: Arnd Bergmann Cc: Cyrille Pitchen, Marek Vasut, Boris Brezillon, Alexandre Torgue, Richard Weinberger, linux-kernel, MTD Maling List, linux-arm-kernel, Maxime Coquelin, Brian Norris, David Woodhouse, Ludovic Barre Hi Arnd, On Thu, Sep 14, 2017 at 1:06 PM, Arnd Bergmann <arnd@arndb.de> wrote: > If we send zero-length data to stm32_qspi_tx_poll() on older > compiler versions such as gcc-4.6, we get warned that the > return code is uninitialized: > > drivers/mtd/spi-nor/stm32-quadspi.c:248:2: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized] > > On newer compiler versions, the return code is always zero > in this case, as the local variable gets optimized away and > is assumed to be zero after the loop completes without error. > > This changes the function to instead return -EINVAL if it > ever gets called with a zero length buffer. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82203 > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/mtd/spi-nor/stm32-quadspi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c > index 86c0931543c5..711cfe7aa4bf 100644 > --- a/drivers/mtd/spi-nor/stm32-quadspi.c > +++ b/drivers/mtd/spi-nor/stm32-quadspi.c > @@ -227,7 +227,7 @@ static int stm32_qspi_tx_poll(struct stm32_qspi *qspi, > void (*tx_fifo)(u8 *, void __iomem *); > u32 len = cmd->len, sr; > u8 *buf = cmd->buf; > - int ret; > + int ret = -EINVAL; > > if (cmd->qspimode == CCR_FMODE_INDW) > tx_fifo = stm32_qspi_write_fifo; See also "[PATCH] mtd: spi-nor: stm32-quadspi: Fix uninitialized error return code" (https://patchwork.kernel.org/patch/9842173/) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: stm32-quadspi: avoid unintialized return code 2017-09-14 13:38 ` Geert Uytterhoeven @ 2017-09-14 15:13 ` Ludovic BARRE 2017-09-14 15:24 ` Geert Uytterhoeven 0 siblings, 1 reply; 8+ messages in thread From: Ludovic BARRE @ 2017-09-14 15:13 UTC (permalink / raw) To: Geert Uytterhoeven, Arnd Bergmann Cc: Cyrille Pitchen, Marek Vasut, Boris Brezillon, Alexandre Torgue, Richard Weinberger, linux-kernel, MTD Maling List, linux-arm-kernel, Maxime Coquelin, Brian Norris, David Woodhouse hi Arnd, Geert On 09/14/2017 03:38 PM, Geert Uytterhoeven wrote: > Hi Arnd, > > On Thu, Sep 14, 2017 at 1:06 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> If we send zero-length data to stm32_qspi_tx_poll() on older >> compiler versions such as gcc-4.6, we get warned that the >> return code is uninitialized: >> >> drivers/mtd/spi-nor/stm32-quadspi.c:248:2: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized] >> >> On newer compiler versions, the return code is always zero >> in this case, as the local variable gets optimized away and >> is assumed to be zero after the loop completes without error. >> >> This changes the function to instead return -EINVAL if it >> ever gets called with a zero length buffer. >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82203 >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- >> drivers/mtd/spi-nor/stm32-quadspi.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c >> index 86c0931543c5..711cfe7aa4bf 100644 >> --- a/drivers/mtd/spi-nor/stm32-quadspi.c >> +++ b/drivers/mtd/spi-nor/stm32-quadspi.c >> @@ -227,7 +227,7 @@ static int stm32_qspi_tx_poll(struct stm32_qspi *qspi, >> void (*tx_fifo)(u8 *, void __iomem *); >> u32 len = cmd->len, sr; >> u8 *buf = cmd->buf; >> - int ret; >> + int ret = -EINVAL; >> >> if (cmd->qspimode == CCR_FMODE_INDW) >> tx_fifo = stm32_qspi_write_fifo; > > See also "[PATCH] mtd: spi-nor: stm32-quadspi: Fix uninitialized error > return code" > (https://patchwork.kernel.org/patch/9842173/) hi Arnd, Geert sorry, I was forgot this thread while my holidays Geert: what do you mean like "similar bugs in the future" in "If you initialized ret at the beginning, you lose the ability to catch newly introduced similar bugs in the future." > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: stm32-quadspi: avoid unintialized return code 2017-09-14 15:13 ` Ludovic BARRE @ 2017-09-14 15:24 ` Geert Uytterhoeven 2017-09-14 16:55 ` Ludovic BARRE 0 siblings, 1 reply; 8+ messages in thread From: Geert Uytterhoeven @ 2017-09-14 15:24 UTC (permalink / raw) To: Ludovic BARRE Cc: Arnd Bergmann, Cyrille Pitchen, Marek Vasut, Boris Brezillon, Alexandre Torgue, Richard Weinberger, linux-kernel, MTD Maling List, linux-arm-kernel, Maxime Coquelin, Brian Norris, David Woodhouse Hi Ludovic, On Thu, Sep 14, 2017 at 5:13 PM, Ludovic BARRE <ludovic.barre@st.com> wrote: > On 09/14/2017 03:38 PM, Geert Uytterhoeven wrote: >> On Thu, Sep 14, 2017 at 1:06 PM, Arnd Bergmann <arnd@arndb.de> wrote: >>> If we send zero-length data to stm32_qspi_tx_poll() on older >>> compiler versions such as gcc-4.6, we get warned that the >>> return code is uninitialized: >>> >>> drivers/mtd/spi-nor/stm32-quadspi.c:248:2: error: ‘ret’ may be used >>> uninitialized in this function [-Werror=uninitialized] >>> >>> On newer compiler versions, the return code is always zero >>> in this case, as the local variable gets optimized away and >>> is assumed to be zero after the loop completes without error. >>> >>> This changes the function to instead return -EINVAL if it >>> ever gets called with a zero length buffer. >>> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82203 >>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >>> --- >>> drivers/mtd/spi-nor/stm32-quadspi.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c >>> b/drivers/mtd/spi-nor/stm32-quadspi.c >>> index 86c0931543c5..711cfe7aa4bf 100644 >>> --- a/drivers/mtd/spi-nor/stm32-quadspi.c >>> +++ b/drivers/mtd/spi-nor/stm32-quadspi.c >>> @@ -227,7 +227,7 @@ static int stm32_qspi_tx_poll(struct stm32_qspi >>> *qspi, >>> void (*tx_fifo)(u8 *, void __iomem *); >>> u32 len = cmd->len, sr; >>> u8 *buf = cmd->buf; >>> - int ret; >>> + int ret = -EINVAL; >>> >>> if (cmd->qspimode == CCR_FMODE_INDW) >>> tx_fifo = stm32_qspi_write_fifo; >> >> >> See also "[PATCH] mtd: spi-nor: stm32-quadspi: Fix uninitialized error >> return code" >> (https://patchwork.kernel.org/patch/9842173/) > > hi Arnd, Geert > > sorry, I was forgot this thread while my holidays > > Geert: what do you mean like "similar bugs in the future" in "If you > initialized ret at the beginning, you lose the ability to catch newly > introduced similar bugs in the future." If you pre-initialize ret at the top, you loose the ability of the compiler to detect at compile-time if ret is never written to later. It will just return -EINVAL at runtime. With my version, if the code is modified later and another "return ret" is added, the compiler will detect if there's a code path that forgets to assign a value to ret. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: stm32-quadspi: avoid unintialized return code 2017-09-14 15:24 ` Geert Uytterhoeven @ 2017-09-14 16:55 ` Ludovic BARRE 2017-09-14 21:38 ` Arnd Bergmann 0 siblings, 1 reply; 8+ messages in thread From: Ludovic BARRE @ 2017-09-14 16:55 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Arnd Bergmann, Cyrille Pitchen, Marek Vasut, Boris Brezillon, Alexandre Torgue, Richard Weinberger, linux-kernel, MTD Maling List, linux-arm-kernel, Maxime Coquelin, Brian Norris, David Woodhouse On 09/14/2017 05:24 PM, Geert Uytterhoeven wrote: > Hi Ludovic, > > On Thu, Sep 14, 2017 at 5:13 PM, Ludovic BARRE <ludovic.barre@st.com> wrote: >> On 09/14/2017 03:38 PM, Geert Uytterhoeven wrote: >>> On Thu, Sep 14, 2017 at 1:06 PM, Arnd Bergmann <arnd@arndb.de> wrote: >>>> If we send zero-length data to stm32_qspi_tx_poll() on older >>>> compiler versions such as gcc-4.6, we get warned that the >>>> return code is uninitialized: >>>> >>>> drivers/mtd/spi-nor/stm32-quadspi.c:248:2: error: ‘ret’ may be used >>>> uninitialized in this function [-Werror=uninitialized] >>>> >>>> On newer compiler versions, the return code is always zero >>>> in this case, as the local variable gets optimized away and >>>> is assumed to be zero after the loop completes without error. >>>> >>>> This changes the function to instead return -EINVAL if it >>>> ever gets called with a zero length buffer. >>>> >>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82203 >>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >>>> --- >>>> drivers/mtd/spi-nor/stm32-quadspi.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c >>>> b/drivers/mtd/spi-nor/stm32-quadspi.c >>>> index 86c0931543c5..711cfe7aa4bf 100644 >>>> --- a/drivers/mtd/spi-nor/stm32-quadspi.c >>>> +++ b/drivers/mtd/spi-nor/stm32-quadspi.c >>>> @@ -227,7 +227,7 @@ static int stm32_qspi_tx_poll(struct stm32_qspi >>>> *qspi, >>>> void (*tx_fifo)(u8 *, void __iomem *); >>>> u32 len = cmd->len, sr; >>>> u8 *buf = cmd->buf; >>>> - int ret; >>>> + int ret = -EINVAL; >>>> >>>> if (cmd->qspimode == CCR_FMODE_INDW) >>>> tx_fifo = stm32_qspi_write_fifo; >>> >>> >>> See also "[PATCH] mtd: spi-nor: stm32-quadspi: Fix uninitialized error >>> return code" >>> (https://patchwork.kernel.org/patch/9842173/) >> >> hi Arnd, Geert >> >> sorry, I was forgot this thread while my holidays >> >> Geert: what do you mean like "similar bugs in the future" in "If you >> initialized ret at the beginning, you lose the ability to catch newly >> introduced similar bugs in the future." > > If you pre-initialize ret at the top, you loose the ability of the compiler > to detect at compile-time if ret is never written to later. It will just return > -EINVAL at runtime. > > With my version, if the code is modified later and another "return ret" is > added, the compiler will detect if there's a code path that forgets > to assign a value to ret. Ok, it's clear for me. I favor geert's solution. Arnd what do you think ? > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: stm32-quadspi: avoid unintialized return code 2017-09-14 16:55 ` Ludovic BARRE @ 2017-09-14 21:38 ` Arnd Bergmann 2017-09-15 7:59 ` Ludovic BARRE 0 siblings, 1 reply; 8+ messages in thread From: Arnd Bergmann @ 2017-09-14 21:38 UTC (permalink / raw) To: Ludovic BARRE Cc: Geert Uytterhoeven, Cyrille Pitchen, Marek Vasut, Boris Brezillon, Alexandre Torgue, Richard Weinberger, linux-kernel, MTD Maling List, linux-arm-kernel, Maxime Coquelin, Brian Norris, David Woodhouse On Thu, Sep 14, 2017 at 6:55 PM, Ludovic BARRE <ludovic.barre@st.com> wrote: > > > On 09/14/2017 05:24 PM, Geert Uytterhoeven wrote: >> >> Hi Ludovic, >> >> On Thu, Sep 14, 2017 at 5:13 PM, Ludovic BARRE <ludovic.barre@st.com> >> wrote: >>> >>> On 09/14/2017 03:38 PM, Geert Uytterhoeven wrote: >>> >>> hi Arnd, Geert >>> >>> sorry, I was forgot this thread while my holidays >>> >>> Geert: what do you mean like "similar bugs in the future" in "If you >>> initialized ret at the beginning, you lose the ability to catch newly >>> introduced similar bugs in the future." >> >> >> If you pre-initialize ret at the top, you loose the ability of the >> compiler >> to detect at compile-time if ret is never written to later. It will just >> return >> -EINVAL at runtime. >> >> With my version, if the code is modified later and another "return ret" is >> added, the compiler will detect if there's a code path that forgets >> to assign a value to ret. > > Ok, it's clear for me. > I favor geert's solution. > Arnd what do you think ? I usually follow the same rule that Geert explained (and quote https://rusty.ozlabs.org/?p=232 when I do so). In this case, there did not seem to be much value as the variable is not used afterwards, and I kept the 'single return statement' guideline. In the end, either version seems totally fine to me here, so please use Geert's if you prefer that. Arnd ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: stm32-quadspi: avoid unintialized return code 2017-09-14 21:38 ` Arnd Bergmann @ 2017-09-15 7:59 ` Ludovic BARRE 0 siblings, 0 replies; 8+ messages in thread From: Ludovic BARRE @ 2017-09-15 7:59 UTC (permalink / raw) To: Arnd Bergmann Cc: Geert Uytterhoeven, Cyrille Pitchen, Marek Vasut, Boris Brezillon, Alexandre Torgue, Richard Weinberger, linux-kernel, MTD Maling List, linux-arm-kernel, Maxime Coquelin, Brian Norris, David Woodhouse On 09/14/2017 11:38 PM, Arnd Bergmann wrote: > On Thu, Sep 14, 2017 at 6:55 PM, Ludovic BARRE <ludovic.barre@st.com> wrote: >> >> >> On 09/14/2017 05:24 PM, Geert Uytterhoeven wrote: >>> >>> Hi Ludovic, >>> >>> On Thu, Sep 14, 2017 at 5:13 PM, Ludovic BARRE <ludovic.barre@st.com> >>> wrote: >>>> >>>> On 09/14/2017 03:38 PM, Geert Uytterhoeven wrote: >>>> >>>> hi Arnd, Geert >>>> >>>> sorry, I was forgot this thread while my holidays >>>> >>>> Geert: what do you mean like "similar bugs in the future" in "If you >>>> initialized ret at the beginning, you lose the ability to catch newly >>>> introduced similar bugs in the future." >>> >>> >>> If you pre-initialize ret at the top, you loose the ability of the >>> compiler >>> to detect at compile-time if ret is never written to later. It will just >>> return >>> -EINVAL at runtime. >>> >>> With my version, if the code is modified later and another "return ret" is >>> added, the compiler will detect if there's a code path that forgets >>> to assign a value to ret. >> >> Ok, it's clear for me. >> I favor geert's solution. >> Arnd what do you think ? > > I usually follow the same rule that Geert explained (and quote > https://rusty.ozlabs.org/?p=232 when I do so). In this case, there > did not seem to be much value as the variable is not used > afterwards, and I kept the 'single return statement' guideline. > > In the end, either version seems totally fine to me here, so > please use Geert's if you prefer that. thank Arnd for your answer, great link :-) we take geert's patch. Geert: I will acked your patch. thanks everybody > > Arnd > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-09-15 8:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-14 11:06 [PATCH] mtd: spi-nor: stm32-quadspi: avoid unintialized return code Arnd Bergmann 2017-09-14 12:38 ` Ludovic BARRE 2017-09-14 13:38 ` Geert Uytterhoeven 2017-09-14 15:13 ` Ludovic BARRE 2017-09-14 15:24 ` Geert Uytterhoeven 2017-09-14 16:55 ` Ludovic BARRE 2017-09-14 21:38 ` Arnd Bergmann 2017-09-15 7:59 ` Ludovic BARRE
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).