[v2] Strict disable implicit fallthrough

[RFC PATCH v2 58/78] hw/ppc: add fallthrough pseudo-keyword

Emmanouil Pitsidianakis posted 78 patches 2 years, 2 months ago

Diff against v1 v3
Download mbox

Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Christian Schoenebeck <qemu_oss@crudebyte.com>, John Snow <jsnow@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>, Ronnie Sahlberg <ronniesahlberg@gmail.com>, Peter Lieven <pl@kamp.de>, Jeff Cody <codyprime@gmail.com>, Yuval Shaia <yuval.shaia.ml@gmail.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Raphael Norwitz <raphael.norwitz@nutanix.com>, "Michael S. Tsirkin" <mst@redhat.com>, Laurent Vivier <laurent@vivier.eu>, Yoshinori Sato <ysato@users.sourceforge.jp>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, "Alex Bennée" <alex.bennee@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, Markus Armbruster <armbru@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Juan Quintela <quintela@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Peter Maydell <peter.maydell@linaro.org>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, "Cédric Le Goater" <clg@kaod.org>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>, Alistair Francis <alistair@alistair23.me>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Eric Auger <eric.auger@redhat.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul@xen.org>, Jonathan Cameron <jonathan.cameron@huawei.com>, Fan Ni <fan.ni@samsung.com>, Peter Xu <peterx@redhat.com>, Jason Wang <jasowang@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, David Woodhouse <dwmw2@infradead.org>, Corey Minyard <minyard@acm.org>, Paul Burton <paulburton@kernel.org>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Pavel Pisa <pisa@cmp.felk.cvut.cz>, Vikram Garhwal <fnu.vikram@xilinx.com>, Akihiko Odaki <akihiko.odaki@daynix.com>, Sriram Yagnaraman <sriram.yagnaraman@est.tech>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Keith Busch <kbusch@kernel.org>, Klaus Jensen <its@irrelevant.dk>, Nicholas Piggin <npiggin@gmail.com>, "Frédéric Barrat" <fbarrat@linux.ibm.com>, Huai-Cheng Kuo <hchkuo@avery-design.com.tw>, Chris Browy <cbrowy@avery-design.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Hannes Reinecke <hare@suse.com>, Bin Meng <bin.meng@windriver.com>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, Magnus Damm <magnus.damm@gmail.com>, Fabiano Rosas <farosas@suse.de>, Leonardo Bras <leobras@redhat.com>, Eric Blake <eblake@redhat.com>, Michael Roth <michael.roth@amd.com>, Konstantin Kostiuk <kkostiuk@redhat.com>, Michael Rolnik <mrolnik@gmail.com>, Alessandro Di Federico <ale@rev.ng>, Anton Johansson <anjo@rev.ng>, Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Marcelo Tosatti <mtosatti@redhat.com>, Song Gao <gaosong@loongson.cn>, Huacai Chen <chenhuacai@kernel.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Chris Wulff <crwulff@gmail.com>, Marek Vasut <marex@denx.de>, Stafford Horne <shorne@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Max Filippov <jcmvbkbc@gmail.com>, WANG Xuerui <git@xen0n.name>, Stefan Weil <sw@weilnetz.de>

There is a newer version of this series

Expand all Fold all

[RFC PATCH v2 58/78] hw/ppc: add fallthrough pseudo-keyword

Posted by Emmanouil Pitsidianakis 2 years, 2 months ago

In preparation of raising -Wimplicit-fallthrough to 5, replace all
fall-through comments with the fallthrough attribute pseudo-keyword.

Signed-off-by: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 hw/ppc/pnv_bmc.c      | 2 +-
 hw/ppc/spapr_events.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
index 99f1e8d7f9..9bff7d03cb 100644
--- a/hw/ppc/pnv_bmc.c
+++ b/hw/ppc/pnv_bmc.c
@@ -172,69 +172,69 @@ static int hiomap_erase(PnvPnor *pnor, uint32_t offset, uint32_t size)
 static void hiomap_cmd(IPMIBmcSim *ibs, uint8_t *cmd, unsigned int cmd_len,
                        RspBuffer *rsp)
 {
     PnvPnor *pnor = PNV_PNOR(object_property_get_link(OBJECT(ibs), "pnor",
                                                       &error_abort));
     uint32_t pnor_size = pnor->size;
     uint32_t pnor_addr = PNOR_SPI_OFFSET;
     bool readonly = false;
 
     rsp_buffer_push(rsp, cmd[2]);
     rsp_buffer_push(rsp, cmd[3]);
 
     switch (cmd[2]) {
     case HIOMAP_C_MARK_DIRTY:
     case HIOMAP_C_FLUSH:
     case HIOMAP_C_ACK:
         break;
 
     case HIOMAP_C_ERASE:
         if (hiomap_erase(pnor, blocks_to_bytes(cmd[5] << 8 | cmd[4]),
                         blocks_to_bytes(cmd[7] << 8 | cmd[6]))) {
             rsp_buffer_set_error(rsp, IPMI_CC_UNSPECIFIED);
         }
         break;
 
     case HIOMAP_C_GET_INFO:
         rsp_buffer_push(rsp, 2);  /* Version 2 */
         rsp_buffer_push(rsp, BLOCK_SHIFT); /* block size */
         rsp_buffer_push(rsp, 0);  /* Timeout */
         rsp_buffer_push(rsp, 0);  /* Timeout */
         break;
 
     case HIOMAP_C_GET_FLASH_INFO:
         rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) & 0xFF);
         rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) >> 8);
         rsp_buffer_push(rsp, 0x01);  /* erase size */
         rsp_buffer_push(rsp, 0x00);  /* erase size */
         break;
 
     case HIOMAP_C_CREATE_READ_WINDOW:
         readonly = true;
-        /* Fall through */
+        fallthrough;
 
     case HIOMAP_C_CREATE_WRITE_WINDOW:
         memory_region_set_readonly(&pnor->mmio, readonly);
         memory_region_set_enabled(&pnor->mmio, true);
 
         rsp_buffer_push(rsp, bytes_to_blocks(pnor_addr) & 0xFF);
         rsp_buffer_push(rsp, bytes_to_blocks(pnor_addr) >> 8);
         rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) & 0xFF);
         rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) >> 8);
         rsp_buffer_push(rsp, 0x00); /* offset */
         rsp_buffer_push(rsp, 0x00); /* offset */
         break;
 
     case HIOMAP_C_CLOSE_WINDOW:
         memory_region_set_enabled(&pnor->mmio, false);
         break;
 
     case HIOMAP_C_DEVICE_NAME:
     case HIOMAP_C_RESET:
     case HIOMAP_C_LOCK:
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "HIOMAP: unknown command %02X\n", cmd[2]);
         break;
     }
 }
 
 #define HIOMAP   0x5a
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 4508e40814..9d51746daf 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -411,25 +411,26 @@ static const SpaprEventSource *
 rtas_event_log_to_source(SpaprMachineState *spapr, int log_type)
 {
     const SpaprEventSource *source;
 
     g_assert(spapr->event_sources);
 
     switch (log_type) {
     case RTAS_LOG_TYPE_HOTPLUG:
         source = spapr_event_sources_get_source(spapr->event_sources,
                                                 EVENT_CLASS_HOT_PLUG);
         if (spapr_ovec_test(spapr->ov5_cas, OV5_HP_EVT)) {
             g_assert(source->enabled);
             break;
         }
         /* fall through back to epow for legacy hotplug interrupt source */
+        fallthrough;
     case RTAS_LOG_TYPE_EPOW:
         source = spapr_event_sources_get_source(spapr->event_sources,
                                                 EVENT_CLASS_EPOW);
         break;
     default:
         source = NULL;
     }
 
     return source;
 }
-- 
2.39.2

Re: [RFC PATCH v2 58/78] hw/ppc: add fallthrough pseudo-keyword

Posted by Harsh Prateek Bora 2 years, 2 months ago


On 10/13/23 13:27, Emmanouil Pitsidianakis wrote:
> In preparation of raising -Wimplicit-fallthrough to 5, replace all
> fall-through comments with the fallthrough attribute pseudo-keyword.
> 
> Signed-off-by: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

One question below, may be Cedric knows better who introduced initial code.

> ---
>   hw/ppc/pnv_bmc.c      | 2 +-
>   hw/ppc/spapr_events.c | 1 +
>   2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
> index 99f1e8d7f9..9bff7d03cb 100644
> --- a/hw/ppc/pnv_bmc.c
> +++ b/hw/ppc/pnv_bmc.c
> @@ -172,69 +172,69 @@ static int hiomap_erase(PnvPnor *pnor, uint32_t offset, uint32_t size)
>   static void hiomap_cmd(IPMIBmcSim *ibs, uint8_t *cmd, unsigned int cmd_len,
>                          RspBuffer *rsp)
>   {
>       PnvPnor *pnor = PNV_PNOR(object_property_get_link(OBJECT(ibs), "pnor",
>                                                         &error_abort));
>       uint32_t pnor_size = pnor->size;
>       uint32_t pnor_addr = PNOR_SPI_OFFSET;
>       bool readonly = false;
>   
>       rsp_buffer_push(rsp, cmd[2]);
>       rsp_buffer_push(rsp, cmd[3]);
>   
>       switch (cmd[2]) {
>       case HIOMAP_C_MARK_DIRTY:
>       case HIOMAP_C_FLUSH:
>       case HIOMAP_C_ACK:
>           break;
>   
>       case HIOMAP_C_ERASE:
>           if (hiomap_erase(pnor, blocks_to_bytes(cmd[5] << 8 | cmd[4]),
>                           blocks_to_bytes(cmd[7] << 8 | cmd[6]))) {
>               rsp_buffer_set_error(rsp, IPMI_CC_UNSPECIFIED);
>           }
>           break;
>   
>       case HIOMAP_C_GET_INFO:
>           rsp_buffer_push(rsp, 2);  /* Version 2 */
>           rsp_buffer_push(rsp, BLOCK_SHIFT); /* block size */
>           rsp_buffer_push(rsp, 0);  /* Timeout */
>           rsp_buffer_push(rsp, 0);  /* Timeout */
>           break;
>   
>       case HIOMAP_C_GET_FLASH_INFO:
>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) & 0xFF);
>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) >> 8);
>           rsp_buffer_push(rsp, 0x01);  /* erase size */
>           rsp_buffer_push(rsp, 0x00);  /* erase size */
>           break;
>   
>       case HIOMAP_C_CREATE_READ_WINDOW:
>           readonly = true;
> -        /* Fall through */
> +        fallthrough;
>   
>       case HIOMAP_C_CREATE_WRITE_WINDOW:
>           memory_region_set_readonly(&pnor->mmio, readonly);
>           memory_region_set_enabled(&pnor->mmio, true);
>   
>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_addr) & 0xFF);
>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_addr) >> 8);
>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) & 0xFF);
>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) >> 8);
>           rsp_buffer_push(rsp, 0x00); /* offset */
>           rsp_buffer_push(rsp, 0x00); /* offset */
>           break;
>   
>       case HIOMAP_C_CLOSE_WINDOW:
>           memory_region_set_enabled(&pnor->mmio, false);
>           break;
>   
>       case HIOMAP_C_DEVICE_NAME:
>       case HIOMAP_C_RESET:
>       case HIOMAP_C_LOCK:

Do we need a break here ?
Otherwise above 3 case statements doesnt add any value.

>       default:
>           qemu_log_mask(LOG_GUEST_ERROR, "HIOMAP: unknown command %02X\n", cmd[2]);
>           break;
>       }
>   }
>   
>   #define HIOMAP   0x5a
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 4508e40814..9d51746daf 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -411,25 +411,26 @@ static const SpaprEventSource *
>   rtas_event_log_to_source(SpaprMachineState *spapr, int log_type)
>   {
>       const SpaprEventSource *source;
>   
>       g_assert(spapr->event_sources);
>   
>       switch (log_type) {
>       case RTAS_LOG_TYPE_HOTPLUG:
>           source = spapr_event_sources_get_source(spapr->event_sources,
>                                                   EVENT_CLASS_HOT_PLUG);
>           if (spapr_ovec_test(spapr->ov5_cas, OV5_HP_EVT)) {
>               g_assert(source->enabled);
>               break;
>           }
>           /* fall through back to epow for legacy hotplug interrupt source */
> +        fallthrough;
>       case RTAS_LOG_TYPE_EPOW:
>           source = spapr_event_sources_get_source(spapr->event_sources,
>                                                   EVENT_CLASS_EPOW);
>           break;
>       default:
>           source = NULL;
>       }
>   
>       return source;
>   }

Re: [RFC PATCH v2 58/78] hw/ppc: add fallthrough pseudo-keyword

Posted by Cédric Le Goater 2 years, 2 months ago

On 10/13/23 10:32, Harsh Prateek Bora wrote:
> 
> 
> On 10/13/23 13:27, Emmanouil Pitsidianakis wrote:
>> In preparation of raising -Wimplicit-fallthrough to 5, replace all
>> fall-through comments with the fallthrough attribute pseudo-keyword.
>>
>> Signed-off-by: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
> 
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> 
> One question below, may be Cedric knows better who introduced initial code.
> 
>> ---
>>   hw/ppc/pnv_bmc.c      | 2 +-
>>   hw/ppc/spapr_events.c | 1 +
>>   2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
>> index 99f1e8d7f9..9bff7d03cb 100644
>> --- a/hw/ppc/pnv_bmc.c
>> +++ b/hw/ppc/pnv_bmc.c
>> @@ -172,69 +172,69 @@ static int hiomap_erase(PnvPnor *pnor, uint32_t offset, uint32_t size)
>>   static void hiomap_cmd(IPMIBmcSim *ibs, uint8_t *cmd, unsigned int cmd_len,
>>                          RspBuffer *rsp)
>>   {
>>       PnvPnor *pnor = PNV_PNOR(object_property_get_link(OBJECT(ibs), "pnor",
>>                                                         &error_abort));
>>       uint32_t pnor_size = pnor->size;
>>       uint32_t pnor_addr = PNOR_SPI_OFFSET;
>>       bool readonly = false;
>>       rsp_buffer_push(rsp, cmd[2]);
>>       rsp_buffer_push(rsp, cmd[3]);
>>       switch (cmd[2]) {
>>       case HIOMAP_C_MARK_DIRTY:
>>       case HIOMAP_C_FLUSH:
>>       case HIOMAP_C_ACK:
>>           break;
>>       case HIOMAP_C_ERASE:
>>           if (hiomap_erase(pnor, blocks_to_bytes(cmd[5] << 8 | cmd[4]),
>>                           blocks_to_bytes(cmd[7] << 8 | cmd[6]))) {
>>               rsp_buffer_set_error(rsp, IPMI_CC_UNSPECIFIED);
>>           }
>>           break;
>>       case HIOMAP_C_GET_INFO:
>>           rsp_buffer_push(rsp, 2);  /* Version 2 */
>>           rsp_buffer_push(rsp, BLOCK_SHIFT); /* block size */
>>           rsp_buffer_push(rsp, 0);  /* Timeout */
>>           rsp_buffer_push(rsp, 0);  /* Timeout */
>>           break;
>>       case HIOMAP_C_GET_FLASH_INFO:
>>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) & 0xFF);
>>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) >> 8);
>>           rsp_buffer_push(rsp, 0x01);  /* erase size */
>>           rsp_buffer_push(rsp, 0x00);  /* erase size */
>>           break;
>>       case HIOMAP_C_CREATE_READ_WINDOW:
>>           readonly = true;
>> -        /* Fall through */
>> +        fallthrough;
>>       case HIOMAP_C_CREATE_WRITE_WINDOW:
>>           memory_region_set_readonly(&pnor->mmio, readonly);
>>           memory_region_set_enabled(&pnor->mmio, true);
>>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_addr) & 0xFF);
>>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_addr) >> 8);
>>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) & 0xFF);
>>           rsp_buffer_push(rsp, bytes_to_blocks(pnor_size) >> 8);
>>           rsp_buffer_push(rsp, 0x00); /* offset */
>>           rsp_buffer_push(rsp, 0x00); /* offset */
>>           break;
>>       case HIOMAP_C_CLOSE_WINDOW:
>>           memory_region_set_enabled(&pnor->mmio, false);
>>           break;
>>       case HIOMAP_C_DEVICE_NAME:
>>       case HIOMAP_C_RESET:
>>       case HIOMAP_C_LOCK:
> 
> Do we need a break here ?
> Otherwise above 3 case statements doesnt add any value.

Indeed. These came from ca661fae81d3 ("ppc/pnv: Add HIOMAP commands").
The RESET command is implemented in skiboot. DEVICE_NAME and LOCK
seem not. Something to check.

Thanks,

C.

> 
>>       default:
>>           qemu_log_mask(LOG_GUEST_ERROR, "HIOMAP: unknown command %02X\n", cmd[2]);
>>           break;
>>       }
>>   }
>>   #define HIOMAP   0x5a
>> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
>> index 4508e40814..9d51746daf 100644
>> --- a/hw/ppc/spapr_events.c
>> +++ b/hw/ppc/spapr_events.c
>> @@ -411,25 +411,26 @@ static const SpaprEventSource *
>>   rtas_event_log_to_source(SpaprMachineState *spapr, int log_type)
>>   {
>>       const SpaprEventSource *source;
>>       g_assert(spapr->event_sources);
>>       switch (log_type) {
>>       case RTAS_LOG_TYPE_HOTPLUG:
>>           source = spapr_event_sources_get_source(spapr->event_sources,
>>                                                   EVENT_CLASS_HOT_PLUG);
>>           if (spapr_ovec_test(spapr->ov5_cas, OV5_HP_EVT)) {
>>               g_assert(source->enabled);
>>               break;
>>           }
>>           /* fall through back to epow for legacy hotplug interrupt source */
>> +        fallthrough;
>>       case RTAS_LOG_TYPE_EPOW:
>>           source = spapr_event_sources_get_source(spapr->event_sources,
>>                                                   EVENT_CLASS_EPOW);
>>           break;
>>       default:
>>           source = NULL;
>>       }
>>       return source;
>>   }