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

[Harsh Prateek Bora]

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.pitsidiana...@linaro.org>

Reviewed-by: Harsh Prateek Bora <hars...@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;

  }