spi: slave: Fix missing break in switch
PATCH 1/1 spi: slave: Fix missing break in switch
Apparently, this code does not actually fall through to the next case because the machine restarts before it has a chance. However, for the sake of maintenance and readability, we better add the missing break statement.
Addresses-Coverity-ID: 1437892 ("Missing break in switch")
--- drivers/spi/spi-slave-system-control.c | 1 + 1 file changed, 1 insertion(+)
drivers/spi/spi-slave-system-control.c +1 -0
@@ -60,6 +60,7 @@ static void spi_slave_system_control_complete(void *arg)case CMD_REBOOT :dev_info (&priv->spi->dev,"Rebooting system...\n" );kernel_restart (NULL ); +break ;case CMD_POWEROFF :dev_info (&priv->spi->dev,"Powering off system...\n" );
Signature · 1 line
2.7.4
Hi Gustavo,
Quoted 16 lines · Gustavo A. R. Silva
Apparently, this code does not actually fall through to the next case because the machine restarts before it has a chance. However, for the sake of maintenance and readability, we better add the missing break statement.
Addresses-Coverity-ID: 1437892 ("Missing break in switch")
Signed-off-by Gustavo A. R. Silva <[email protected]>--- drivers/spi/spi-slave-system-control.c | 1 + 1 file changed, 1 insertion(+)drivers/spi/spi-slave-system-control.c +0
-0
drivers/spi/spi-slave-system-control.c static void spi_slave_system_control_complete(void *arg) · line 66
Quoted 7 lines
drivers/spi/spi-slave-system-control.c +1
-0@@ -60,6 +60,7 @@ static void spi_slave_system_control_complete(void *arg)case CMD_REBOOT :dev_info (&priv->spi->dev,"Rebooting system...\n" );kernel_restart (NULL ); +break ;case CMD_POWEROFF :dev_info (&priv->spi->dev,"Powering off system...\n" );
Alternatively, kernel_restart() and friends could be marked __noreturn.
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] 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
Mark Brown
maintainerQuoted 8 lines · Geert Uytterhoeven
On Wed, Oct 3, 2018 at 2:57 PM Gustavo A. R. Silva
Quoted 4 lines
case CMD_REBOOT: dev_info(&priv->spi->dev, "Rebooting system...\n"); kernel_restart(NULL);+ break;
Alternatively,
kernel_restart()and friends could be marked__noreturn.
Yes, that seems more sensible though there's no harm in this patch even with that. It'd definitely avoid other issues in future.
Hi,
Quoted 8 lines · Mark Brown
Quoted 6 lines · Geert Uytterhoeven
On Wed, Oct 3, 2018 at 2:57 PM Gustavo A. R. Silva
Quoted 4 lines
case CMD_REBOOT: dev_info(&priv->spi->dev, "Rebooting system...\n"); kernel_restart(NULL);+ break;
Quoted 5 lines
Quoted 1 line
Alternatively,
kernel_restart()and friends could be marked__noreturn.Yes, that seems more sensible though there's no harm in this patch even with that. It'd definitely avoid other issues in future.
I'll include the __noreturn in addition to the break statement. I'll send v2 shortly.
Thank you both for the feedback.
--
Gustavo
Hi Gustavo,
Quoted 9 lines · Gustavo A. R. Silva
Quoted 8 lines · Mark Brown
Quoted 6 lines · Geert Uytterhoeven
On Wed, Oct 3, 2018 at 2:57 PM Gustavo A. R. Silva
Quoted 4 lines
case CMD_REBOOT: dev_info(&priv->spi->dev, "Rebooting system...\n"); kernel_restart(NULL);+ break;
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Alternatively,
kernel_restart()and friends could be marked__noreturn.Yes, that seems more sensible though there's no harm in this patch even with that. It'd definitely avoid other issues in future.
I'll include the
__noreturnin addition to the break statement. I'll send v2 shortly.
Please note that adding __noreturn is not a trivial task, due to the complex call chains, and the different implementations on the various architectures and platforms. So that will be a big patch series.
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] 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
Quoted 18 lines · Geert Uytterhoeven
Hi Gustavo,
Quoted 14 lines · Gustavo A. R. Silva
Quoted 12 lines · Mark Brown
Quoted 8 lines · Geert Uytterhoeven
On Wed, Oct 3, 2018 at 2:57 PM Gustavo A. R. Silva
Quoted 4 lines
case CMD_REBOOT: dev_info(&priv->spi->dev, "Rebooting system...\n"); kernel_restart(NULL);+ break;
Alternatively,
kernel_restart()and friends could be marked__noreturn.Yes, that seems more sensible though there's no harm in this patch even with that. It'd definitely avoid other issues in future.
Quoted 7 lines
Quoted 2 lines
I'll include the
__noreturnin addition to the break statement. I'll send v2 shortly.Please note that adding
__noreturnis not a trivial task, due to the complex call chains, and the different implementations on the various architectures and platforms. So that will be a big patch series.
I see. In that case, it might be an interesting side project.
I appreciate the feedback, Geert.
--
Gustavo
Mark Brown
maintainerQuoted 2 lines · Gustavo A. R. Silva
I'll include the
__noreturnin addition to the break statement. I'll send v2 shortly.
No need for a v2, I already applied this - adding __noreturn seems like a separate (although desirable) effort.