[PATCH v2 0/6] Fix more GCC9 -O3 warnings

[PATCH v2 0/6] Fix more GCC9 -O3 warnings

[Philippe Mathieu-Daudé]

Fix some trivial warnings when building with -O3.

v2:
- addressed Thomas and Aleksandar comments
- dropped 'hw/scsi/megasas: Silent GCC9 duplicated-cond warning'
- dropped 'qemu-io-cmds: Silent GCC9 format-overflow warning'

Philippe Mathieu-Daudé (6):
  audio/audio: Add missing fall through comment
  hw/display/tcx: Add missing fall through comments
  hw/timer/aspeed_timer: Add a fall through comment
  hw/net/imx_fec: Rewrite fall through comments
  hw/net/imx_fec: Remove unuseful FALLTHROUGH comments
  hw/pci-host/designware: Remove unuseful FALLTHROUGH comment

 audio/audio.c            | 1 +
 hw/display/tcx.c         | 2 ++
 hw/net/imx_fec.c         | 7 ++++---
 hw/pci-host/designware.c | 2 +-
 hw/timer/aspeed_timer.c  | 2 +-
 5 files changed, 9 insertions(+), 5 deletions(-)

Cc: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: "Cédric Le Goater" <clg@kaod.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Joel Stanley <joel@jms.id.au>
Cc: "Kővágó, Zoltán" <dirty.ice.hu@gmail.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Olivier Danet <odanet@caramail.com>
Cc: Peter Chubb <peter.chubb@nicta.com.au>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
-- 
2.21.0

[PATCH v2 1/6] audio/audio: Add missing fall through comment

[Philippe Mathieu-Daudé]

When building with GCC9 using CFLAG -Wimplicit-fallthrough=2 we get:

  audio/audio.c: In function ‘audio_pcm_init_info’:
  audio/audio.c:306:14: error: this statement may fall through [-Werror=implicit-fallthrough=]
    306 |         sign = 1;
        |         ~~~~~^~~
  audio/audio.c:307:5: note: here
    307 |     case AUDIO_FORMAT_U8:
        |     ^~~~
  cc1: all warnings being treated as errors

Similarly to e46349414, add the missing fall through comment to
hint GCC.

Fixes: 2b9cce8c8c
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Reword the description (Aleksandar)

Cc: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: "Kővágó, Zoltán" <dirty.ice.hu@gmail.com>
---
 audio/audio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/audio/audio.c b/audio/audio.c
index 56fae55047..57daf3f620 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -304,6 +304,7 @@ void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as)
     switch (as->fmt) {
     case AUDIO_FORMAT_S8:
         sign = 1;
+        /* fall through */
     case AUDIO_FORMAT_U8:
         mul = 1;
         break;
-- 
2.21.0

[PATCH v2 2/6] hw/display/tcx: Add missing fall through comments

[Philippe Mathieu-Daudé]

When building with GCC9 using CFLAG -Wimplicit-fallthrough=2 we get:

  hw/display/tcx.c: In function ‘tcx_dac_writel’:
  hw/display/tcx.c:453:26: error: this statement may fall through [-Werror=implicit-fallthrough=]
    453 |             s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
        |             ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
  hw/display/tcx.c:454:9: note: here
    454 |         default:
        |         ^~~~~~~
  hw/display/tcx.c: In function ‘tcx_dac_readl’:
  hw/display/tcx.c:412:22: error: this statement may fall through [-Werror=implicit-fallthrough=]
    412 |         s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
        |         ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
  hw/display/tcx.c:413:5: note: here
    413 |     default:
        |     ^~~~~~~
  cc1: all warnings being treated as errors

Give a hint to GCC by adding the missing fall through comments.

Fixes: 55d7bfe22
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Reword the description (Aleksandar)

Cc: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
Cc: Olivier Danet <odanet@caramail.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/display/tcx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 14e829d3fa..abbeb30284 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -410,6 +410,7 @@ static uint64_t tcx_dac_readl(void *opaque, hwaddr addr,
     case 2:
         val = s->b[s->dac_index] << 24;
         s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
+        /* fall through */
     default:
         s->dac_state = 0;
         break;
@@ -451,6 +452,7 @@ static void tcx_dac_writel(void *opaque, hwaddr addr, uint64_t val,
             s->b[index] = val >> 24;
             update_palette_entries(s, index, index + 1);
             s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
+            /* fall through */
         default:
             s->dac_state = 0;
             break;
-- 
2.21.0

[PATCH v2 3/6] hw/timer/aspeed_timer: Add a fall through comment

[Philippe Mathieu-Daudé]

Reported by GCC9 when building with CFLAG -Wimplicit-fallthrough=2:

  hw/timer/aspeed_timer.c: In function ‘aspeed_timer_set_value’:
  hw/timer/aspeed_timer.c:283:24: error: this statement may fall through [-Werror=implicit-fallthrough=]
    283 |         if (old_reload || !t->reload) {
        |             ~~~~~~~~~~~^~~~~~~~~~~~~
  hw/timer/aspeed_timer.c:287:5: note: here
    287 |     case TIMER_REG_STATUS:
        |     ^~~~
  cc1: all warnings being treated as errors

Add the missing fall through comment.

Fixes: 1403f364472
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Cc: "Cédric Le Goater" <clg@kaod.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Joel Stanley <joel@jms.id.au>
Cc: qemu-arm@nongnu.org
---
 hw/timer/aspeed_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
index a8c38cc118..c91f18415c 100644
--- a/hw/timer/aspeed_timer.c
+++ b/hw/timer/aspeed_timer.c
@@ -283,7 +283,7 @@ static void aspeed_timer_set_value(AspeedTimerCtrlState *s, int timer, int reg,
         if (old_reload || !t->reload) {
             break;
         }
-
+        /* fall through to re-enable */
     case TIMER_REG_STATUS:
         if (timer_enabled(t)) {
             uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-- 
2.21.0

[PATCH v2 4/6] hw/net/imx_fec: Rewrite fall through comments

[Philippe Mathieu-Daudé]

GCC9 is confused by this comment when building with CFLAG
-Wimplicit-fallthrough=2:

  hw/net/imx_fec.c: In function ‘imx_eth_write’:
  hw/net/imx_fec.c:906:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
    906 |         if (unlikely(single_tx_ring)) {
        |            ^
  hw/net/imx_fec.c:912:5: note: here
    912 |     case ENET_TDAR:     /* FALLTHROUGH */
        |     ^~~~
  cc1: all warnings being treated as errors

Rewrite the comments in the correct place,  using 'fall through'
which is recognized by GCC and static analyzers.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Cc: Peter Chubb <peter.chubb@nicta.com.au>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Jason Wang <jasowang@redhat.com>
Cc: qemu-arm@nongnu.org
---
 hw/net/imx_fec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index bd99236864..c01ce4f078 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -909,7 +909,8 @@ static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value,
                           TYPE_IMX_FEC, __func__);
             return;
         }
-    case ENET_TDAR:     /* FALLTHROUGH */
+        /* fall through */
+    case ENET_TDAR:
         if (s->regs[ENET_ECR] & ENET_ECR_ETHEREN) {
             s->regs[index] = ENET_TDAR_TDAR;
             imx_eth_do_tx(s, index);
-- 
2.21.0

[RFC PATCH v2 5/6] hw/net/imx_fec: Remove unuseful FALLTHROUGH comments

[Philippe Mathieu-Daudé]

We don't need to explicit these obvious switch fall through
comments. Stay consistent with the rest of the codebase.

Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Cc: Peter Chubb <peter.chubb@nicta.com.au>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Jason Wang <jasowang@redhat.com>
Cc: qemu-arm@nongnu.org
---
 hw/net/imx_fec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index c01ce4f078..5a83678f64 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -901,8 +901,8 @@ static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value,
             s->regs[index] = 0;
         }
         break;
-    case ENET_TDAR1:    /* FALLTHROUGH */
-    case ENET_TDAR2:    /* FALLTHROUGH */
+    case ENET_TDAR1:
+    case ENET_TDAR2:
         if (unlikely(single_tx_ring)) {
             qemu_log_mask(LOG_GUEST_ERROR,
                           "[%s]%s: trying to access TDAR2 or TDAR1\n",
-- 
2.21.0

[RFC PATCH v2 6/6] hw/pci-host/designware: Remove unuseful FALLTHROUGH comment

[Philippe Mathieu-Daudé]

We don't need to explicit this obvious switch fall through.
Stay consistent with the rest of the codebase.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Cc: Peter Chubb <peter.chubb@nicta.com.au>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: qemu-arm@nongnu.org
---
 hw/pci-host/designware.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 71e9b0d9b5..dd245516dd 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -182,7 +182,7 @@ designware_pcie_root_config_read(PCIDevice *d, uint32_t address, int len)
         break;
 
     case DESIGNWARE_PCIE_ATU_CR1:
-    case DESIGNWARE_PCIE_ATU_CR2:          /* FALLTHROUGH */
+    case DESIGNWARE_PCIE_ATU_CR2:
         val = viewport->cr[(address - DESIGNWARE_PCIE_ATU_CR1) /
                            sizeof(uint32_t)];
         break;
-- 
2.21.0

Re: [RFC PATCH v2 5/6] hw/net/imx_fec: Remove unuseful FALLTHROUGH comments

[Thomas Huth]

On 18/12/2019 20.25, Philippe Mathieu-Daudé wrote:
> We don't need to explicit these obvious switch fall through
> comments. Stay consistent with the rest of the codebase.
> 
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
...
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index c01ce4f078..5a83678f64 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -901,8 +901,8 @@ static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value,
>              s->regs[index] = 0;
>          }
>          break;
> -    case ENET_TDAR1:    /* FALLTHROUGH */
> -    case ENET_TDAR2:    /* FALLTHROUGH */
> +    case ENET_TDAR1:
> +    case ENET_TDAR2:
>          if (unlikely(single_tx_ring)) {
>              qemu_log_mask(LOG_GUEST_ERROR,
>                            "[%s]%s: trying to access TDAR2 or TDAR1\n",

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [RFC PATCH v2 6/6] hw/pci-host/designware: Remove unuseful FALLTHROUGH comment

[Thomas Huth]

On 18/12/2019 20.25, Philippe Mathieu-Daudé wrote:
> We don't need to explicit this obvious switch fall through.
> Stay consistent with the rest of the codebase.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: Peter Chubb <peter.chubb@nicta.com.au>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: qemu-arm@nongnu.org
> ---
>  hw/pci-host/designware.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 71e9b0d9b5..dd245516dd 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -182,7 +182,7 @@ designware_pcie_root_config_read(PCIDevice *d, uint32_t address, int len)
>          break;
>  
>      case DESIGNWARE_PCIE_ATU_CR1:
> -    case DESIGNWARE_PCIE_ATU_CR2:          /* FALLTHROUGH */
> +    case DESIGNWARE_PCIE_ATU_CR2:
>          val = viewport->cr[(address - DESIGNWARE_PCIE_ATU_CR1) /
>                             sizeof(uint32_t)];
>          break;
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [RFC PATCH v2 6/6] hw/pci-host/designware: Remove unuseful FALLTHROUGH comment

[Aleksandar Markovic]

On Wed, Dec 18, 2019 at 8:29 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> We don't need to explicit this obvious switch fall through.
> Stay consistent with the rest of the codebase.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

> Cc: Peter Chubb <peter.chubb@nicta.com.au>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: qemu-arm@nongnu.org
> ---
>  hw/pci-host/designware.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 71e9b0d9b5..dd245516dd 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -182,7 +182,7 @@ designware_pcie_root_config_read(PCIDevice *d, uint32_t address, int len)
>          break;
>
>      case DESIGNWARE_PCIE_ATU_CR1:
> -    case DESIGNWARE_PCIE_ATU_CR2:          /* FALLTHROUGH */
> +    case DESIGNWARE_PCIE_ATU_CR2:
>          val = viewport->cr[(address - DESIGNWARE_PCIE_ATU_CR1) /
>                             sizeof(uint32_t)];
>          break;
> --
> 2.21.0
>
>

Re: [RFC PATCH v2 5/6] hw/net/imx_fec: Remove unuseful FALLTHROUGH comments

[Aleksandar Markovic]

On Wed, Dec 18, 2019 at 8:29 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> We don't need to explicit these obvious switch fall through
> comments. Stay consistent with the rest of the codebase.
>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

> Cc: Peter Chubb <peter.chubb@nicta.com.au>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: qemu-arm@nongnu.org
> ---
>  hw/net/imx_fec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index c01ce4f078..5a83678f64 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -901,8 +901,8 @@ static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value,
>              s->regs[index] = 0;
>          }
>          break;
> -    case ENET_TDAR1:    /* FALLTHROUGH */
> -    case ENET_TDAR2:    /* FALLTHROUGH */
> +    case ENET_TDAR1:
> +    case ENET_TDAR2:
>          if (unlikely(single_tx_ring)) {
>              qemu_log_mask(LOG_GUEST_ERROR,
>                            "[%s]%s: trying to access TDAR2 or TDAR1\n",
> --
> 2.21.0
>
>

Re: [PATCH v2 4/6] hw/net/imx_fec: Rewrite fall through comments

[Thomas Huth]

On 18/12/2019 20.25, Philippe Mathieu-Daudé wrote:
> GCC9 is confused by this comment when building with CFLAG
> -Wimplicit-fallthrough=2:
> 
>   hw/net/imx_fec.c: In function ‘imx_eth_write’:
>   hw/net/imx_fec.c:906:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
>     906 |         if (unlikely(single_tx_ring)) {
>         |            ^
>   hw/net/imx_fec.c:912:5: note: here
>     912 |     case ENET_TDAR:     /* FALLTHROUGH */
>         |     ^~~~
>   cc1: all warnings being treated as errors
> 
> Rewrite the comments in the correct place,  using 'fall through'
> which is recognized by GCC and static analyzers.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: Peter Chubb <peter.chubb@nicta.com.au>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: qemu-arm@nongnu.org
> ---
>  hw/net/imx_fec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index bd99236864..c01ce4f078 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -909,7 +909,8 @@ static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value,
>                            TYPE_IMX_FEC, __func__);
>              return;
>          }
> -    case ENET_TDAR:     /* FALLTHROUGH */
> +        /* fall through */
> +    case ENET_TDAR:
>          if (s->regs[ENET_ECR] & ENET_ECR_ETHEREN) {
>              s->regs[index] = ENET_TDAR_TDAR;
>              imx_eth_do_tx(s, index);
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v2 1/6] audio/audio: Add missing fall through comment

[Aleksandar Markovic]

On Wed, Dec 18, 2019 at 8:26 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> When building with GCC9 using CFLAG -Wimplicit-fallthrough=2 we get:
>
>   audio/audio.c: In function ‘audio_pcm_init_info’:
>   audio/audio.c:306:14: error: this statement may fall through [-Werror=implicit-fallthrough=]
>     306 |         sign = 1;
>         |         ~~~~~^~~
>   audio/audio.c:307:5: note: here
>     307 |     case AUDIO_FORMAT_U8:
>         |     ^~~~
>   cc1: all warnings being treated as errors
>
> Similarly to e46349414, add the missing fall through comment to
> hint GCC.
>
> Fixes: 2b9cce8c8c
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: Reword the description (Aleksandar)
>

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

> Cc: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: "Kővágó, Zoltán" <dirty.ice.hu@gmail.com>
> ---
>  audio/audio.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 56fae55047..57daf3f620 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -304,6 +304,7 @@ void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as)
>      switch (as->fmt) {
>      case AUDIO_FORMAT_S8:
>          sign = 1;
> +        /* fall through */
>      case AUDIO_FORMAT_U8:
>          mul = 1;
>          break;
> --
> 2.21.0
>

Re: [PATCH v2 2/6] hw/display/tcx: Add missing fall through comments

[Aleksandar Markovic]

On Wed, Dec 18, 2019 at 8:26 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> When building with GCC9 using CFLAG -Wimplicit-fallthrough=2 we get:
>
>   hw/display/tcx.c: In function ‘tcx_dac_writel’:
>   hw/display/tcx.c:453:26: error: this statement may fall through [-Werror=implicit-fallthrough=]
>     453 |             s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
>         |             ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   hw/display/tcx.c:454:9: note: here
>     454 |         default:
>         |         ^~~~~~~
>   hw/display/tcx.c: In function ‘tcx_dac_readl’:
>   hw/display/tcx.c:412:22: error: this statement may fall through [-Werror=implicit-fallthrough=]
>     412 |         s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
>         |         ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   hw/display/tcx.c:413:5: note: here
>     413 |     default:
>         |     ^~~~~~~
>   cc1: all warnings being treated as errors
>
> Give a hint to GCC by adding the missing fall through comments.
>
> Fixes: 55d7bfe22
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: Reword the description (Aleksandar)
>

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

> Cc: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
> Cc: Olivier Danet <odanet@caramail.com>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/display/tcx.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 14e829d3fa..abbeb30284 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -410,6 +410,7 @@ static uint64_t tcx_dac_readl(void *opaque, hwaddr addr,
>      case 2:
>          val = s->b[s->dac_index] << 24;
>          s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
> +        /* fall through */
>      default:
>          s->dac_state = 0;
>          break;
> @@ -451,6 +452,7 @@ static void tcx_dac_writel(void *opaque, hwaddr addr, uint64_t val,
>              s->b[index] = val >> 24;
>              update_palette_entries(s, index, index + 1);
>              s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
> +            /* fall through */
>          default:
>              s->dac_state = 0;
>              break;
> --
> 2.21.0
>

Re: [RFC PATCH v2 5/6] hw/net/imx_fec: Remove unuseful FALLTHROUGH comments

[Richard Henderson]

On 12/18/19 9:25 AM, Philippe Mathieu-Daudé wrote:
> We don't need to explicit these obvious switch fall through
> comments. Stay consistent with the rest of the codebase.
> 
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: Peter Chubb <peter.chubb@nicta.com.au>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: qemu-arm@nongnu.org
> ---
>  hw/net/imx_fec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [RFC PATCH v2 6/6] hw/pci-host/designware: Remove unuseful FALLTHROUGH comment

[Richard Henderson]

On 12/18/19 9:25 AM, Philippe Mathieu-Daudé wrote:
> We don't need to explicit this obvious switch fall through.
> Stay consistent with the rest of the codebase.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: Peter Chubb <peter.chubb@nicta.com.au>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: qemu-arm@nongnu.org
> ---
>  hw/pci-host/designware.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [RFC PATCH v2 6/6] hw/pci-host/designware: Remove unuseful FALLTHROUGH comment

[Aleksandar Markovic]

On Wed, Dec 18, 2019 at 8:29 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> We don't need to explicit this obvious switch fall through.
> Stay consistent with the rest of the codebase.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---

I see you covered two cases of such comment (in patches 5 and 6).

But why didn't you than cover the following cases:

block/vhdx.c:            case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
block/vhdx.c:            case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */

hw/net/rtl8139.c:    case 1: /* fall through */
hw/net/rtl8139.c:    case 2: /* fall through */

contrib/vhost-user-scsi/vhost-user-scsi.c:    case 1: /* fall through */
(this one needs full switch block reformatting, but can this be
changed, since it is in "contrib"?)

?

Thanks,
Aleksandar



> Cc: Peter Chubb <peter.chubb@nicta.com.au>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: qemu-arm@nongnu.org
> ---
>  hw/pci-host/designware.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 71e9b0d9b5..dd245516dd 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -182,7 +182,7 @@ designware_pcie_root_config_read(PCIDevice *d, uint32_t address, int len)
>          break;
>
>      case DESIGNWARE_PCIE_ATU_CR1:
> -    case DESIGNWARE_PCIE_ATU_CR2:          /* FALLTHROUGH */
> +    case DESIGNWARE_PCIE_ATU_CR2:
>          val = viewport->cr[(address - DESIGNWARE_PCIE_ATU_CR1) /
>                             sizeof(uint32_t)];
>          break;
> --
> 2.21.0
>
>

Re: [PATCH v2 2/6] hw/display/tcx: Add missing fall through comments

[Mark Cave-Ayland]

On 18/12/2019 19:25, Philippe Mathieu-Daudé wrote:

> When building with GCC9 using CFLAG -Wimplicit-fallthrough=2 we get:
> 
>   hw/display/tcx.c: In function ‘tcx_dac_writel’:
>   hw/display/tcx.c:453:26: error: this statement may fall through [-Werror=implicit-fallthrough=]
>     453 |             s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
>         |             ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   hw/display/tcx.c:454:9: note: here
>     454 |         default:
>         |         ^~~~~~~
>   hw/display/tcx.c: In function ‘tcx_dac_readl’:
>   hw/display/tcx.c:412:22: error: this statement may fall through [-Werror=implicit-fallthrough=]
>     412 |         s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
>         |         ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   hw/display/tcx.c:413:5: note: here
>     413 |     default:
>         |     ^~~~~~~
>   cc1: all warnings being treated as errors
> 
> Give a hint to GCC by adding the missing fall through comments.
> 
> Fixes: 55d7bfe22
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: Reword the description (Aleksandar)
> 
> Cc: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
> Cc: Olivier Danet <odanet@caramail.com>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/display/tcx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 14e829d3fa..abbeb30284 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -410,6 +410,7 @@ static uint64_t tcx_dac_readl(void *opaque, hwaddr addr,
>      case 2:
>          val = s->b[s->dac_index] << 24;
>          s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
> +        /* fall through */
>      default:
>          s->dac_state = 0;
>          break;
> @@ -451,6 +452,7 @@ static void tcx_dac_writel(void *opaque, hwaddr addr, uint64_t val,
>              s->b[index] = val >> 24;
>              update_palette_entries(s, index, index + 1);
>              s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
> +            /* fall through */
>          default:
>              s->dac_state = 0;
>              break;

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [PATCH v2 1/6] audio/audio: Add missing fall through comment

[Gerd Hoffmann]

> diff --git a/audio/audio.c b/audio/audio.c
> index 56fae55047..57daf3f620 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -304,6 +304,7 @@ void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as)
>      switch (as->fmt) {
>      case AUDIO_FORMAT_S8:
>          sign = 1;
> +        /* fall through */
>      case AUDIO_FORMAT_U8:

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

Re: [PATCH v2 0/6] Fix more GCC9 -O3 warnings

[Philippe Mathieu-Daudé]

Cc'ing qemu-trivial as this series is fully reviewed.

On 12/18/19 8:25 PM, Philippe Mathieu-Daudé wrote:
> Fix some trivial warnings when building with -O3.
> 
> v2:
> - addressed Thomas and Aleksandar comments
> - dropped 'hw/scsi/megasas: Silent GCC9 duplicated-cond warning'
> - dropped 'qemu-io-cmds: Silent GCC9 format-overflow warning'
> 
> Philippe Mathieu-Daudé (6):
>    audio/audio: Add missing fall through comment
>    hw/display/tcx: Add missing fall through comments
>    hw/timer/aspeed_timer: Add a fall through comment
>    hw/net/imx_fec: Rewrite fall through comments
>    hw/net/imx_fec: Remove unuseful FALLTHROUGH comments
>    hw/pci-host/designware: Remove unuseful FALLTHROUGH comment
> 
>   audio/audio.c            | 1 +
>   hw/display/tcx.c         | 2 ++
>   hw/net/imx_fec.c         | 7 ++++---
>   hw/pci-host/designware.c | 2 +-
>   hw/timer/aspeed_timer.c  | 2 +-
>   5 files changed, 9 insertions(+), 5 deletions(-)
> 
> Cc: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: "Cédric Le Goater" <clg@kaod.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Joel Stanley <joel@jms.id.au>
> Cc: "Kővágó, Zoltán" <dirty.ice.hu@gmail.com>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Olivier Danet <odanet@caramail.com>
> Cc: Peter Chubb <peter.chubb@nicta.com.au>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
>