qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] Annotate questionable fallthroughs


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH] Annotate questionable fallthroughs
Date: Mon, 21 Jan 2013 14:11:43 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux)

Blue Swirl <address@hidden> writes:

> Recent Clang compilers have preliminary support for finding
> unannotated fallthrough cases in switch statements with
> compiler flag -Wimplicit-fallthrough. The support is incomplete,
> it's only possible to annotate the case in C++ but not in C, so it
> wouldn't be useful to enable the flag for QEMU yet.
>
> Mark cases which don't have a comment about fall through with
> a comment. In legitimate fall through cases the comment can be
> edited later to mark the case for future readers.

Let's clean this up properly instead, as far as we can.  Details inline.
Maintainers, please check out the parts that apply to your code.

> Signed-off-by: Blue Swirl <address@hidden>
> ---
>  audio/audio.c                |    3 ++
>  disas/cris.c                 |    1 +
>  disas/m68k.c                 |    1 +
>  disas/sh4.c                  |    2 +
>  hw/arm_sysctl.c              |    2 +
>  hw/cadence_ttc.c             |    2 +
>  hw/cirrus_vga.c              |    1 +
>  hw/es1370.c                  |   20 +++++++++++++++
>  hw/hid.c                     |    2 +
>  hw/highbank.c                |    2 +
>  hw/ide/core.c                |    8 ++++++
>  hw/jazz_led.c                |    1 +
>  hw/omap1.c                   |    3 ++
>  hw/omap_dma.c                |   12 +++++++++
>  hw/omap_spi.c                |   24 ++++++++++++++++++
>  hw/pflash_cfi02.c            |    1 +
>  hw/ppc.c                     |    1 +
>  hw/pxa2xx.c                  |    2 +
>  hw/pxa2xx_timer.c            |   47 ++++++++++++++++++++++++++++++++++++
>  hw/scsi-bus.c                |    2 +
>  hw/sh_timer.c                |    5 ++++
>  hw/smc91c111.c               |    1 +
>  hw/stellaris.c               |    2 +
>  hw/tcx.c                     |    1 +
>  hw/twl92230.c                |   17 +++++++++++++
>  hw/usb/hcd-ohci.c            |    2 +
>  linux-user/main.c            |    4 +++
>  linux-user/syscall.c         |    1 +
>  target-i386/translate.c      |    3 ++
>  target-mips/translate.c      |   54 
> ++++++++++++++++++++++++++++++++++++++++++
>  target-ppc/mmu_helper.c      |    1 +
>  target-s390x/translate.c     |    1 +
>  target-sparc/ldst_helper.c   |    4 +++
>  target-unicore32/translate.c |    2 +
>  target-xtensa/op_helper.c    |    2 +
>  tcg/optimize.c               |    3 ++
>  ui/sdl.c                     |    1 +
>  37 files changed, 241 insertions(+), 0 deletions(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 02bb886..b42489b 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -617,11 +617,13 @@ void audio_pcm_init_info (struct audio_pcm_info *info, 
> struct audsettings *as)
>      switch (as->fmt) {
>      case AUD_FMT_S8:
>          sign = 1;
> +        /* XXX: questionable fallthrough */
>      case AUD_FMT_U8:
>          break;
>  
>      case AUD_FMT_S16:
>          sign = 1;
> +        /* XXX: questionable fallthrough */
>      case AUD_FMT_U16:
>          bits = 16;
>          shift = 1;
> @@ -629,6 +631,7 @@ void audio_pcm_init_info (struct audio_pcm_info *info, 
> struct audsettings *as)
>  
>      case AUD_FMT_S32:
>          sign = 1;
> +        /* XXX: questionable fallthrough */
>      case AUD_FMT_U32:
>          bits = 32;
>          shift = 2;

Similar code in audio_pcm_info_eq() already has fall through comments.
Code is maintained.  Care to post a patch?

> diff --git a/disas/cris.c b/disas/cris.c
> index 9dfb4e3..c2c08fa 100644
> --- a/disas/cris.c
> +++ b/disas/cris.c
> @@ -1348,6 +1348,7 @@ spec_reg_info (unsigned int sreg, enum 
> cris_disass_family distype)
>               /* No ambiguous sizes or register names with CRISv32.  */
>               if (cris_spec_regs[i].warning == NULL)
>                 return &cris_spec_regs[i];
> +                /* XXX: questionable fallthrough */
>             default:
>               ;
>             }

Inherited from binutils; if you want to clean this up, suggest to do it
there.

> diff --git a/disas/m68k.c b/disas/m68k.c
> index c950241..7e82046 100644
> --- a/disas/m68k.c
> +++ b/disas/m68k.c
> @@ -1626,6 +1626,7 @@ print_insn_arg (const char *d,
>  
>      case 'X':
>        place = '8';
> +      /* XXX: questionable fallthrough */
>      case 'Y':
>      case 'Z':
>      case 'W':

Likewise.

> diff --git a/disas/sh4.c b/disas/sh4.c
> index f6cadd5..0e94424 100644
> --- a/disas/sh4.c
> +++ b/disas/sh4.c
> @@ -1969,6 +1969,7 @@ print_insn_sh (bfd_vma memaddr, struct disassemble_info 
> *info)
>                 fprintf_fn (stream, "xd%d", rn & ~1);
>                 break;
>               }
> +              /* XXX: questionable fallthrough */
>           case D_REG_N:
>             fprintf_fn (stream, "dr%d", rn);
>             break;
> @@ -1978,6 +1979,7 @@ print_insn_sh (bfd_vma memaddr, struct disassemble_info 
> *info)
>                 fprintf_fn (stream, "xd%d", rm & ~1);
>                 break;
>               }
> +              /* XXX: questionable fallthrough */
>           case D_REG_M:
>             fprintf_fn (stream, "dr%d", rm);
>             break;

No idea, and SH4 is "odd fixes".

> diff --git a/hw/arm_sysctl.c b/hw/arm_sysctl.c
> index a196fcc..2066ef3 100644
> --- a/hw/arm_sysctl.c
> +++ b/hw/arm_sysctl.c
> @@ -199,6 +199,7 @@ static void arm_sysctl_write(void *opaque, hwaddr offset,
>      switch (offset) {
>      case 0x08: /* LED */
>          s->leds = val;
> +        /* XXX: questionable fallthrough */

Can just as well break here, because:

>      case 0x0c: /* OSC0 */
>      case 0x10: /* OSC1 */
>      case 0x14: /* OSC2 */
       case 0x18: /* OSC3 */
       case 0x1c: /* OSC4 */
           /* ??? */
           break;

> @@ -295,6 +296,7 @@ static void arm_sysctl_write(void *opaque, hwaddr offset,
>              /* On VExpress this register is unimplemented and will RAZ/WI */
>              break;
>          }
> +        /* XXX: questionable fallthrough */

Same here.

>      case 0x54: /* CLCDSER */
>      case 0x64: /* DMAPSR0 */
>      case 0x68: /* DMAPSR1 */
       case 0x6c: /* DMAPSR2 */
       case 0x70: /* IOSEL */
       case 0x74: /* PLDCTL */
       case 0x80: /* BUSID */
       case 0x84: /* PROCID0 */
       case 0x88: /* PROCID1 */
       case 0x8c: /* OSCRESET0 */
       case 0x90: /* OSCRESET1 */
       case 0x94: /* OSCRESET2 */
       case 0x98: /* OSCRESET3 */
       case 0x9c: /* OSCRESET4 */
           break;

Care to post a patch?

> diff --git a/hw/cadence_ttc.c b/hw/cadence_ttc.c
> index 2a8fadd..8613b85 100644
> --- a/hw/cadence_ttc.c
> +++ b/hw/cadence_ttc.c
> @@ -342,11 +342,13 @@ static void cadence_ttc_write(void *opaque, hwaddr 
> offset,
>      case 0x38:
>          s->reg_match[0] = value & 0xffff;
>  
> +        /* XXX: questionable fallthrough */
>      case 0x3c: /* match register */
>      case 0x40:
>      case 0x44:
>          s->reg_match[1] = value & 0xffff;
>  
> +        /* XXX: questionable fallthrough */
>      case 0x48: /* match register */
>      case 0x4c:
>      case 0x50:

Unclear.  Ask the maintainer, Peter Crosthwaite?

> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
> index 2a2c8da..edd68f0 100644
> --- a/hw/cirrus_vga.c
> +++ b/hw/cirrus_vga.c
> @@ -1305,6 +1305,7 @@ static void cirrus_vga_write_sr(CirrusVGAState * s, 
> uint32_t val)
>       break;
>      case 0x07:                       // Extended Sequencer Mode
>      cirrus_update_memory_access(s);
> +    /* XXX: questionable fallthrough */
>      case 0x08:                       // EEPROM Control
>      case 0x09:                       // Scratch Register 0
>      case 0x0a:                       // Scratch Register 1

Commit 2bec46dc makes me suspect this one's intentional.

> diff --git a/hw/es1370.c b/hw/es1370.c
> index 977d2e3..e5793b1 100644
> --- a/hw/es1370.c
> +++ b/hw/es1370.c
> @@ -537,8 +537,10 @@ IO_WRITE_PROTO (es1370_writew)
>  
>      case ES1370_REG_ADC_SCOUNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC2_SCOUNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC1_SCOUNT:
>          d->scount = (d->scount & ~0xffff) | (val & 0xffff);
>          break;
> @@ -574,8 +576,10 @@ IO_WRITE_PROTO (es1370_writel)
>  
>      case ES1370_REG_ADC_SCOUNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC2_SCOUNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC1_SCOUNT:
>          d->scount = (val & 0xffff) | (d->scount & ~0xffff);
>          ldebug ("chan %td CURR_SAMP_CT %d, SAMP_CT %d\n",
> @@ -584,8 +588,10 @@ IO_WRITE_PROTO (es1370_writel)
>  
>      case ES1370_REG_ADC_FRAMEADR:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC2_FRAMEADR:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC1_FRAMEADR:
>          d->frame_addr = val;
>          ldebug ("chan %td frame address %#x\n", d - &s->chan[0], val);
> @@ -600,8 +606,10 @@ IO_WRITE_PROTO (es1370_writel)
>  
>      case ES1370_REG_ADC_FRAMECNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC2_FRAMECNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC1_FRAMECNT:
>          d->frame_cnt = val;
>          d->leftover = 0;
> @@ -661,24 +669,30 @@ IO_READ_PROTO (es1370_readw)
>      switch (addr) {
>      case ES1370_REG_ADC_SCOUNT + 2:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC2_SCOUNT + 2:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC1_SCOUNT + 2:
>          val = d->scount >> 16;
>          break;
>  
>      case ES1370_REG_ADC_FRAMECNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC2_FRAMECNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC1_FRAMECNT:
>          val = d->frame_cnt & 0xffff;
>          break;
>  
>      case ES1370_REG_ADC_FRAMECNT + 2:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC2_FRAMECNT + 2:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC1_FRAMECNT + 2:
>          val = d->frame_cnt >> 16;
>          break;
> @@ -719,8 +733,10 @@ IO_READ_PROTO (es1370_readl)
>  
>      case ES1370_REG_ADC_SCOUNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC2_SCOUNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC1_SCOUNT:
>          val = d->scount;
>  #ifdef DEBUG_ES1370
> @@ -737,8 +753,10 @@ IO_READ_PROTO (es1370_readl)
>  
>      case ES1370_REG_ADC_FRAMECNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC2_FRAMECNT:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC1_FRAMECNT:
>          val = d->frame_cnt;
>  #ifdef DEBUG_ES1370
> @@ -755,8 +773,10 @@ IO_READ_PROTO (es1370_readl)
>  
>      case ES1370_REG_ADC_FRAMEADR:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC2_FRAMEADR:
>          d++;
> +        /* XXX: questionable fallthrough */
>      case ES1370_REG_DAC1_FRAMEADR:
>          val = d->frame_addr;
>          break;

I know nothing about this device, but here goes anyway: these look
intentional to me.

> diff --git a/hw/hid.c b/hw/hid.c
> index 89b5415..f7a815c 100644
> --- a/hw/hid.c
> +++ b/hw/hid.c
> @@ -196,11 +196,13 @@ static void hid_keyboard_process_keycode(HIDState *hs)
>              hs->kbd.modifiers ^= 3 << 8;
>              return;
>          }
> +        /* XXX: questionable fallthrough */
>      case 0xe1 ... 0xe7:
>          if (keycode & (1 << 7)) {
>              hs->kbd.modifiers &= ~(1 << (hid_code & 0x0f));
>              return;
>          }
> +        /* XXX: questionable fallthrough */
>      case 0xe8 ... 0xef:
>          hs->kbd.modifiers |= 1 << (hid_code & 0x0f);
>          return;

These look intentional to me.  Gerd?

> diff --git a/hw/highbank.c b/hw/highbank.c
> index 7bc6b44..28771b1 100644
> --- a/hw/highbank.c
> +++ b/hw/highbank.c
> @@ -70,8 +70,10 @@ static void hb_reset_secondary(ARMCPU *cpu, const struct 
> arm_boot_info *info)
>      switch (info->nb_cpus) {
>      case 4:
>          stl_phys_notdirty(SMP_BOOT_REG + 0x30, 0);
> +        /* XXX: questionable fallthrough */
>      case 3:
>          stl_phys_notdirty(SMP_BOOT_REG + 0x20, 0);
> +        /* XXX: questionable fallthrough */
>      case 2:
>          stl_phys_notdirty(SMP_BOOT_REG + 0x10, 0);
>          env->regs[15] = SMP_BOOT_ADDR;

Unclear.  Ask the maintainer, Mark Langsdorf?

> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 14ad079..0457c65 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -1151,6 +1151,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>          break;
>      case WIN_VERIFY_EXT:
>       lba48 = 1;
> +        /* XXX: questionable fallthrough */
>      case WIN_VERIFY:
>      case WIN_VERIFY_ONCE:
>          /* do sector number check ? */
> @@ -1160,6 +1161,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>          break;
>      case WIN_READ_EXT:
>       lba48 = 1;
> +        /* XXX: questionable fallthrough */
>      case WIN_READ:
>      case WIN_READ_ONCE:
>          if (s->drive_kind == IDE_CD) {
> @@ -1175,6 +1177,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>          break;
>      case WIN_WRITE_EXT:
>       lba48 = 1;
> +        /* XXX: questionable fallthrough */
>      case WIN_WRITE:
>      case WIN_WRITE_ONCE:
>      case CFA_WRITE_SECT_WO_ERASE:
> @@ -1191,6 +1194,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>          break;
>      case WIN_MULTREAD_EXT:
>       lba48 = 1;
> +        /* XXX: questionable fallthrough */
>      case WIN_MULTREAD:
>          if (!s->bs) {
>              goto abort_cmd;
> @@ -1204,6 +1208,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>          break;
>      case WIN_MULTWRITE_EXT:
>       lba48 = 1;
> +        /* XXX: questionable fallthrough */
>      case WIN_MULTWRITE:
>      case CFA_WRITE_MULTI_WO_ERASE:
>          if (!s->bs) {
> @@ -1224,6 +1229,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>          break;
>      case WIN_READDMA_EXT:
>       lba48 = 1;
> +        /* XXX: questionable fallthrough */
>      case WIN_READDMA:
>      case WIN_READDMA_ONCE:
>          if (!s->bs) {
> @@ -1234,6 +1240,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>          break;
>      case WIN_WRITEDMA_EXT:
>       lba48 = 1;
> +        /* XXX: questionable fallthrough */
>      case WIN_WRITEDMA:
>      case WIN_WRITEDMA_ONCE:
>          if (!s->bs) {
> @@ -1245,6 +1252,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>          break;
>      case WIN_READ_NATIVE_MAX_EXT:
>       lba48 = 1;
> +        /* XXX: questionable fallthrough */
>      case WIN_READ_NATIVE_MAX:
>       ide_cmd_lba48_transform(s, lba48);
>          ide_set_sector(s, s->nb_sectors - 1);

These look intentional to me.  Care to post a patch?

> diff --git a/hw/jazz_led.c b/hw/jazz_led.c
> index 4822c48..8b6a16b 100644
> --- a/hw/jazz_led.c
> +++ b/hw/jazz_led.c
> @@ -165,6 +165,7 @@ static void jazz_led_update_display(void *opaque)
>              case 16:
>                  color_segment = rgb_to_pixel16(0xaa, 0xaa, 0xaa);
>                  color_led = rgb_to_pixel16(0x00, 0xff, 0x00);
> +                /* XXX: questionable fallthrough */
>              case 24:
>                  color_segment = rgb_to_pixel24(0xaa, 0xaa, 0xaa);
>                  color_led = rgb_to_pixel24(0x00, 0xff, 0x00);

This one looks wrong to me.  The other cases all break.

> diff --git a/hw/omap1.c b/hw/omap1.c
> index e85f2e2..3e3e5e7 100644
> --- a/hw/omap1.c
> +++ b/hw/omap1.c
> @@ -529,6 +529,7 @@ static uint64_t omap_ulpd_pm_read(void *opaque, hwaddr 
> addr,
>      case 0x28:       /* Reserved */
>      case 0x2c:       /* Reserved */
>          OMAP_BAD_REG(addr);
> +        /* XXX: questionable fallthrough */
>      case 0x00:       /* COUNTER_32_LSB */
>      case 0x04:       /* COUNTER_32_MSB */
>      case 0x08:       /* COUNTER_HIGH_FREQ_LSB */
> @@ -633,6 +634,7 @@ static void omap_ulpd_pm_write(void *opaque, hwaddr addr,
>      case 0x28:       /* Reserved */
>      case 0x2c:       /* Reserved */
>          OMAP_BAD_REG(addr);
> +        /* XXX: questionable fallthrough */
>      case 0x24:       /* SETUP_ANALOG_CELL3_ULPD1 */
>      case 0x38:       /* COUNTER_32_FIQ */
>      case 0x48:       /* LOCL_TIME */
> @@ -1089,6 +1091,7 @@ static void omap_mpui_write(void *opaque, hwaddr addr,
>      /* Not in OMAP310 */
>      case 0x14:       /* DSP_STATUS */
>          OMAP_RO_REG(addr);
> +        /* XXX: questionable fallthrough */
>      case 0x18:       /* DSP_BOOT_CONFIG */
>      case 0x1c:       /* DSP_MPUI_CONFIG */
>          break;

Leave to maintainer, Peter Maydell?

> diff --git a/hw/omap_dma.c b/hw/omap_dma.c
> index aec5874..dbc26e3 100644
> --- a/hw/omap_dma.c
> +++ b/hw/omap_dma.c
> @@ -1709,19 +1709,25 @@ static uint64_t omap_dma4_read(void *opaque, hwaddr 
> addr,
>  
>      case 0x14:       /* DMA4_IRQSTATUS_L3 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x10:       /* DMA4_IRQSTATUS_L2 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x0c:       /* DMA4_IRQSTATUS_L1 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x08:       /* DMA4_IRQSTATUS_L0 */
>          return s->irqstat[irqn];
>  
>      case 0x24:       /* DMA4_IRQENABLE_L3 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x20:       /* DMA4_IRQENABLE_L2 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x1c:       /* DMA4_IRQENABLE_L1 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x18:       /* DMA4_IRQENABLE_L0 */
>          return s->irqen[irqn];
>  
> @@ -1856,10 +1862,13 @@ static void omap_dma4_write(void *opaque, hwaddr addr,
>      switch (addr) {
>      case 0x14:       /* DMA4_IRQSTATUS_L3 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x10:       /* DMA4_IRQSTATUS_L2 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x0c:       /* DMA4_IRQSTATUS_L1 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x08:       /* DMA4_IRQSTATUS_L0 */
>          s->irqstat[irqn] &= ~value;
>          if (!s->irqstat[irqn])
> @@ -1868,10 +1877,13 @@ static void omap_dma4_write(void *opaque, hwaddr addr,
>  
>      case 0x24:       /* DMA4_IRQENABLE_L3 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x20:       /* DMA4_IRQENABLE_L2 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x1c:       /* DMA4_IRQENABLE_L1 */
>          irqn ++;
> +        /* XXX: questionable fallthrough */
>      case 0x18:       /* DMA4_IRQENABLE_L0 */
>          s->irqen[irqn] = value;
>          return;

Similar "cleverness" as in hw/es1370.c.  Peter?

> diff --git a/hw/omap_spi.c b/hw/omap_spi.c
> index 42d5149..6869148 100644
> --- a/hw/omap_spi.c
> +++ b/hw/omap_spi.c
> @@ -167,32 +167,47 @@ static uint64_t omap_mcspi_read(void *opaque, hwaddr 
> addr,
>          return s->control;
>  
>      case 0x68: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x54: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x40: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x2c:       /* MCSPI_CHCONF */
>          return s->ch[ch].config;
>  
>      case 0x6c: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x58: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x44: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x30:       /* MCSPI_CHSTAT */
>          return s->ch[ch].status;
>  
>      case 0x70: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x5c: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x48: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x34:       /* MCSPI_CHCTRL */
>          return s->ch[ch].control;
>  
>      case 0x74: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x60: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x4c: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x38:       /* MCSPI_TX */
>          return s->ch[ch].tx;
>  
>      case 0x78: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x64: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x50: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x3c:       /* MCSPI_RX */
>          s->ch[ch].status &= ~(1 << 0);                       /* RXS */
>          ret = s->ch[ch].rx;
> @@ -269,8 +284,11 @@ static void omap_mcspi_write(void *opaque, hwaddr addr,
>          break;
>  
>      case 0x68: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x54: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x40: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x2c:       /* MCSPI_CHCONF */
>          if ((value ^ s->ch[ch].config) & (3 << 14))  /* DMAR | DMAW */
>              omap_mcspi_dmarequest_update(s->ch + ch);
> @@ -283,8 +301,11 @@ static void omap_mcspi_write(void *opaque, hwaddr addr,
>          break;
>  
>      case 0x70: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x5c: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x48: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x34:       /* MCSPI_CHCTRL */
>          if (value & ~s->ch[ch].control & 1) {                /* EN */
>              s->ch[ch].control |= 1;
> @@ -294,8 +315,11 @@ static void omap_mcspi_write(void *opaque, hwaddr addr,
>          break;
>  
>      case 0x74: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x60: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x4c: ch ++;
> +        /* XXX: questionable fallthrough */
>      case 0x38:       /* MCSPI_TX */
>          s->ch[ch].tx = value;
>          s->ch[ch].status &= ~(1 << 1);                       /* TXS */

Likewise.

> diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c
> index cfb91cb..3c205f0 100644
> --- a/hw/pflash_cfi02.c
> +++ b/hw/pflash_cfi02.c
> @@ -157,6 +157,7 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
>          DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
>          pfl->wcycle = 0;
>          pfl->cmd = 0;
> +        /* XXX: questionable fallthrough */
>      case 0x80:
>          /* We accept reads during second unlock sequence... */
>      case 0x00:

Suspicious.  Peter?

> diff --git a/hw/ppc.c b/hw/ppc.c
> index c52e22f..d22e4d4 100644
> --- a/hw/ppc.c
> +++ b/hw/ppc.c
> @@ -97,6 +97,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>              } else {
>                  cpu_ppc_tb_stop(env);
>              }
> +            /* XXX: questionable fallthrough */
>          case PPC6xx_INPUT_INT:
>              /* Level sensitive - active high */
>              LOG_IRQ("%s: set the external IRQ state to %d\n",

Suspicious.  Could Alexander Graf help?

> diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
> index 492805f..25554a5 100644
> --- a/hw/pxa2xx.c
> +++ b/hw/pxa2xx.c
> @@ -415,6 +415,7 @@ static uint64_t pxa2xx_mm_read(void *opaque, hwaddr addr,
>          if ((addr & 3) == 0)
>              return s->mm_regs[addr >> 2];
>  
> +        /* XXX: questionable fallthrough */
>      default:
>          printf("%s: Bad register " REG_FMT "\n", __FUNCTION__, addr);
>          break;
> @@ -434,6 +435,7 @@ static void pxa2xx_mm_write(void *opaque, hwaddr addr,
>              break;
>          }
>  
> +        /* XXX: questionable fallthrough */
>      default:
>          printf("%s: Bad register " REG_FMT "\n", __FUNCTION__, addr);
>          break;

Unclear.  Could Andrzej Zaborowski help?

> diff --git a/hw/pxa2xx_timer.c b/hw/pxa2xx_timer.c
> index 32c1872..f73bb3f 100644
> --- a/hw/pxa2xx_timer.c
> +++ b/hw/pxa2xx_timer.c
> @@ -157,17 +157,27 @@ static uint64_t pxa2xx_timer_read(void *opaque, hwaddr 
> offset,
>  
>      switch (offset) {
>      case OSMR3:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR2:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR1:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR0:
>          return s->timer[tm].value;
>      case OSMR11: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR10: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR9:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR8:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR7:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR6:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR5:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR4:
>          if (!pxa2xx_timer_has_tm4(s))
>              goto badreg;
> @@ -176,12 +186,19 @@ static uint64_t pxa2xx_timer_read(void *opaque, hwaddr 
> offset,
>          return s->clock + muldiv64(qemu_get_clock_ns(vm_clock) -
>                          s->lastload, s->freq, get_ticks_per_sec());
>      case OSCR11: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR10: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR9:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR8:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR7:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR6:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR5:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR4:
>          if (!pxa2xx_timer_has_tm4(s))
>              goto badreg;
> @@ -207,12 +224,19 @@ static uint64_t pxa2xx_timer_read(void *opaque, hwaddr 
> offset,
>      case OWER:
>          return s->reset3;
>      case OMCR11: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR10: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR9:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR8:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR7:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR6:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR5:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR4:
>          if (!pxa2xx_timer_has_tm4(s))
>              goto badreg;
> @@ -235,19 +259,29 @@ static void pxa2xx_timer_write(void *opaque, hwaddr 
> offset,
>  
>      switch (offset) {
>      case OSMR3:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR2:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR1:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR0:
>          s->timer[tm].value = value;
>          pxa2xx_timer_update(s, qemu_get_clock_ns(vm_clock));
>          break;
>      case OSMR11: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR10: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR9:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR8:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR7:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR6:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR5:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSMR4:
>          if (!pxa2xx_timer_has_tm4(s))
>              goto badreg;
> @@ -261,12 +295,19 @@ static void pxa2xx_timer_write(void *opaque, hwaddr 
> offset,
>          pxa2xx_timer_update(s, s->lastload);
>          break;
>      case OSCR11: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR10: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR9:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR8:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR7:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR6:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR5:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OSCR4:
>          if (!pxa2xx_timer_has_tm4(s))
>              goto badreg;
> @@ -291,8 +332,11 @@ static void pxa2xx_timer_write(void *opaque, hwaddr 
> offset,
>          s->reset3 = value;
>          break;
>      case OMCR7:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR6:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR5:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR4:
>          if (!pxa2xx_timer_has_tm4(s))
>              goto badreg;
> @@ -306,8 +350,11 @@ static void pxa2xx_timer_write(void *opaque, hwaddr 
> offset,
>          }
>          break;
>      case OMCR11: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR10: tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR9:  tm ++;
> +        /* XXX: questionable fallthrough */
>      case OMCR8:  tm += 4;
>          if (!pxa2xx_timer_has_tm4(s))
>              goto badreg;

Similar "cleverness" as in hw/es1370.c.  Andrzej?

> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index 267a942..d70170d 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -888,6 +888,7 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice 
> *dev, uint8_t *buf)
>          if (cmd->xfer == 0) {
>              cmd->xfer = 256;
>          }
> +        /* XXX: questionable fallthrough */
>      case WRITE_10:
>      case WRITE_VERIFY_10:
>      case WRITE_12:
> @@ -902,6 +903,7 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice 
> *dev, uint8_t *buf)
>          if (cmd->xfer == 0) {
>              cmd->xfer = 256;
>          }
> +        /* XXX: questionable fallthrough */
>      case READ_10:
>      case RECOVER_BUFFERED_DATA:
>      case READ_12:

These look intentional to me.  Care to post a patch?

> diff --git a/hw/sh_timer.c b/hw/sh_timer.c
> index 64ea23f..d6f15b1 100644
> --- a/hw/sh_timer.c
> +++ b/hw/sh_timer.c
> @@ -73,6 +73,7 @@ static uint32_t sh_timer_read(void *opaque, hwaddr offset)
>      case OFFSET_TCPR:
>          if (s->feat & TIMER_FEAT_CAPT)
>              return s->tcpr;
> +        /* XXX: questionable fallthrough */
>      default:
>          hw_error("sh_timer_read: Bad offset %x\n", (int)offset);
>          return 0;
> @@ -111,6 +112,7 @@ static void sh_timer_write(void *opaque, hwaddr offset,
>          case 4: freq >>= 10; break;
>       case 6:
>       case 7: if (s->feat & TIMER_FEAT_EXTCLK) break;
> +            /* XXX: questionable fallthrough */
>       default: hw_error("sh_timer_write: Reserved TPSC value\n"); break;
>          }
>          switch ((value & TIMER_TCR_CKEG) >> 3) {
> @@ -118,12 +120,14 @@ static void sh_timer_write(void *opaque, hwaddr offset,
>          case 1:
>          case 2:
>          case 3: if (s->feat & TIMER_FEAT_EXTCLK) break;
> +            /* XXX: questionable fallthrough */
>       default: hw_error("sh_timer_write: Reserved CKEG value\n"); break;
>          }
>          switch ((value & TIMER_TCR_ICPE) >> 6) {
>       case 0: break;
>          case 2:
>          case 3: if (s->feat & TIMER_FEAT_CAPT) break;
> +            /* XXX: questionable fallthrough */
>       default: hw_error("sh_timer_write: Reserved ICPE value\n"); break;
>          }
>       if ((value & TIMER_TCR_UNF) == 0)
> @@ -151,6 +155,7 @@ static void sh_timer_write(void *opaque, hwaddr offset,
>              s->tcpr = value;
>           break;
>       }
> +        /* XXX: questionable fallthrough */
>      default:
>          hw_error("sh_timer_write: Bad offset %x\n", (int)offset);
>      }

Could be intentional.  Who could know for sure?  SH4 is "odd fixes".

> diff --git a/hw/smc91c111.c b/hw/smc91c111.c
> index a34698f..08d6829 100644
> --- a/hw/smc91c111.c
> +++ b/hw/smc91c111.c
> @@ -442,6 +442,7 @@ static void smc91c111_writeb(void *opaque, hwaddr offset,
>              return;
>          case 12: /* Early receive.  */
>              s->ercv = value & 0x1f;
> +            /* XXX: questionable fallthrough */

Could just as well return here.  Care to post a patch?

>          case 13:
>              /* Ignore.  */
>              return;
> diff --git a/hw/stellaris.c b/hw/stellaris.c
> index b5e78ff..a2e2457 100644
> --- a/hw/stellaris.c
> +++ b/hw/stellaris.c
> @@ -182,8 +182,10 @@ static uint64_t gptm_read(void *opaque, hwaddr offset,
>      case 0x48: /* TAR */
>          if (s->control == 1)
>              return s->rtc;
> +        /* XXX: questionable fallthrough */

Could be intentional.  Paul Brook?

>      case 0x4c: /* TBR */
>          hw_error("TODO: Timer value read\n");
> +        /* XXX: questionable fallthrough */

Nope, hw_error() never returns.

>      default:
>          hw_error("gptm_read: Bad offset 0x%x\n", (int)offset);
>          return 0;
> diff --git a/hw/tcx.c b/hw/tcx.c
> index 0ce2952..889c5c9 100644
> --- a/hw/tcx.c
> +++ b/hw/tcx.c
> @@ -465,6 +465,7 @@ static void tcx_dac_writel(void *opaque, hwaddr addr, 
> uint64_t val,
>              s->b[s->dac_index] = val >> 24;
>              update_palette_entries(s, s->dac_index, s->dac_index + 1);
>              s->dac_index = (s->dac_index + 1) & 255; // Index autoincrement
> +            /* XXX: questionable fallthrough */
>          default:
>              s->dac_state = 0;
>              break;

Suspicious.  Paul Brook?

> diff --git a/hw/twl92230.c b/hw/twl92230.c
> index 70d9b03..dfa1c51 100644
> --- a/hw/twl92230.c
> +++ b/hw/twl92230.c
> @@ -268,28 +268,42 @@ static uint8_t menelaus_read(void *opaque, uint8_t addr)
>          return 0x22;
>  
>      case MENELAUS_VCORE_CTRL5: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_VCORE_CTRL4: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_VCORE_CTRL3: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_VCORE_CTRL2: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_VCORE_CTRL1:
>          return s->vcore[reg];
>  
>      case MENELAUS_DCDC_CTRL3: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_DCDC_CTRL2: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_DCDC_CTRL1:
>          return s->dcdc[reg];
>  
>      case MENELAUS_LDO_CTRL8: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_LDO_CTRL7: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_LDO_CTRL6: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_LDO_CTRL5: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_LDO_CTRL4: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_LDO_CTRL3: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_LDO_CTRL2: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_LDO_CTRL1:
>          return s->ldo[reg];
>  
>      case MENELAUS_SLEEP_CTRL2: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_SLEEP_CTRL1:
>          return s->sleep[reg];
>  
> @@ -386,7 +400,9 @@ static uint8_t menelaus_read(void *opaque, uint8_t addr)
>          return s->pull[3];
>  
>      case MENELAUS_MCT_CTRL3: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_MCT_CTRL2: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_MCT_CTRL1:
>          return s->mmc_ctrl[reg];
>      case MENELAUS_MCT_PIN_ST:
> @@ -487,6 +503,7 @@ static void menelaus_write(void *opaque, uint8_t addr, 
> uint8_t value)
>          break;
>  
>      case MENELAUS_SLEEP_CTRL2: reg ++;
> +        /* XXX: questionable fallthrough */
>      case MENELAUS_SLEEP_CTRL1:
>          s->sleep[reg] = value;
>          break;

Similar "cleverness" as in hw/es1370.c.  Andrzej?

> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index 6a2f5f8..8004f6b 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -1103,6 +1103,7 @@ static int ohci_service_td(OHCIState *ohci, struct 
> ohci_ed *ed)
>              case USB_RET_IOERROR:
>              case USB_RET_NODEV:
>                  OHCI_SET_BM(td.flags, TD_CC, OHCI_CC_DEVICENOTRESPONDING);
> +                /* XXX: questionable fallthrough */
>              case USB_RET_NAK:
>                  DPRINTF("usb-ohci: got NAK\n");
>                  return 1;

Suspicious.  At the very least, the DPRINTF() is misleading.  Gerd?

> @@ -1737,6 +1738,7 @@ static void ohci_mem_write(void *opaque,
>      case 24: /* HcStatus */
>          ohci->hstatus &= ~(val & ohci->hmask);
>  
> +        /* XXX: questionable fallthrough */
>      case 25: /* HcHReset */
>          ohci->hreset = val & ~OHCI_HRESET_FSBIR;
>          if (val & OHCI_HRESET_FSBIR)

Also suspicious.  Gerd?

> diff --git a/linux-user/main.c b/linux-user/main.c
> index 0181bc2..f0cbcc2 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -2211,18 +2211,22 @@ void cpu_loop(CPUMIPSState *env)
>                      if ((ret = get_user_ual(arg8, sp_reg + 28)) != 0) {
>                          goto done_syscall;
>                      }
> +                    /* XXX: questionable fallthrough */
>                  case 7:
>                      if ((ret = get_user_ual(arg7, sp_reg + 24)) != 0) {
>                          goto done_syscall;
>                      }
> +                    /* XXX: questionable fallthrough */
>                  case 6:
>                      if ((ret = get_user_ual(arg6, sp_reg + 20)) != 0) {
>                          goto done_syscall;
>                      }
> +                    /* XXX: questionable fallthrough */
>                  case 5:
>                      if ((ret = get_user_ual(arg5, sp_reg + 16)) != 0) {
>                          goto done_syscall;
>                      }
> +                    /* XXX: questionable fallthrough */
>                  default:
>                      break;
>                  }

No idea.  Riku Voipio?

> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 693e66f..f20632d 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8458,6 +8458,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>        goto unimplemented_nowarn;
>  #endif
>  #endif
> +      /* XXX: questionable fallthrough */
>  #ifdef TARGET_NR_get_thread_area
>      case TARGET_NR_get_thread_area:
>  #if defined(TARGET_I386) && defined(TARGET_ABI32)

Inpenetrable #ifdef thicket.  Riku?

> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index 32d21f5..be0fc85 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -3797,6 +3797,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
> int b,
>          case 0x138:
>              if (s->prefix & PREFIX_REPNZ)
>                  goto crc32;
> +            /* XXX: questionable fallthrough */
>          case 0x038:
>              b = modrm;
>              modrm = cpu_ldub_code(env, s->pc++);
> @@ -4412,6 +4413,7 @@ static target_ulong disas_insn(CPUX86State *env, 
> DisasContext *s,
>      case 0x82:
>          if (CODE64(s))
>              goto illegal_op;
> +        /* XXX: questionable fallthrough */
>      case 0x80: /* GRP1 */
>      case 0x81:
>      case 0x83:
> @@ -7790,6 +7792,7 @@ static target_ulong disas_insn(CPUX86State *env, 
> DisasContext *s,
>      case 0x10e ... 0x10f:
>          /* 3DNow! instructions, ignore prefixes */
>          s->prefix &= ~(PREFIX_REPZ | PREFIX_REPNZ | PREFIX_DATA);
> +        /* XXX: questionable fallthrough */
>      case 0x110 ... 0x117:
>      case 0x128 ... 0x12f:
>      case 0x138 ... 0x13a:

These look intentional to me.  Care to post a patch?

> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 206ba83..9762695 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -4243,6 +4243,7 @@ static void gen_mfc0 (CPUMIPSState *env, DisasContext 
> *ctx, TCGv arg, int reg, i
>  //            gen_helper_mfc0_contextconfig(arg); /* SmartMIPS ASE */
>              rn = "ContextConfig";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -4523,18 +4524,22 @@ static void gen_mfc0 (CPUMIPSState *env, DisasContext 
> *ctx, TCGv arg, int reg, i
>  //            gen_helper_mfc0_tracecontrol(arg); /* PDtrace support */
>              rn = "TraceControl";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 2:
>  //            gen_helper_mfc0_tracecontrol2(arg); /* PDtrace support */
>              rn = "TraceControl2";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 3:
>  //            gen_helper_mfc0_usertracedata(arg); /* PDtrace support */
>              rn = "UserTraceData";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 4:
>  //            gen_helper_mfc0_tracebpc(arg); /* PDtrace support */
>              rn = "TraceBPC";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -4561,30 +4566,37 @@ static void gen_mfc0 (CPUMIPSState *env, DisasContext 
> *ctx, TCGv arg, int reg, i
>  //            gen_helper_mfc0_performance1(arg);
>              rn = "Performance1";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 2:
>  //            gen_helper_mfc0_performance2(arg);
>              rn = "Performance2";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 3:
>  //            gen_helper_mfc0_performance3(arg);
>              rn = "Performance3";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 4:
>  //            gen_helper_mfc0_performance4(arg);
>              rn = "Performance4";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 5:
>  //            gen_helper_mfc0_performance5(arg);
>              rn = "Performance5";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 6:
>  //            gen_helper_mfc0_performance6(arg);
>              rn = "Performance6";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 7:
>  //            gen_helper_mfc0_performance7(arg);
>              rn = "Performance7";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -4823,6 +4835,7 @@ static void gen_mtc0 (CPUMIPSState *env, DisasContext 
> *ctx, TCGv arg, int reg, i
>  //            gen_helper_mtc0_contextconfig(cpu_env, arg); /* SmartMIPS ASE 
> */
>              rn = "ContextConfig";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -5105,12 +5118,14 @@ static void gen_mtc0 (CPUMIPSState *env, DisasContext 
> *ctx, TCGv arg, int reg, i
>              /* Stop translation as we may have switched the execution mode */
>              ctx->bstate = BS_STOP;
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 2:
>  //            gen_helper_mtc0_tracecontrol2(cpu_env, arg); /* PDtrace 
> support */
>              rn = "TraceControl2";
>              /* Stop translation as we may have switched the execution mode */
>              ctx->bstate = BS_STOP;
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 3:
>              /* Stop translation as we may have switched the execution mode */
>              ctx->bstate = BS_STOP;
> @@ -5119,12 +5134,14 @@ static void gen_mtc0 (CPUMIPSState *env, DisasContext 
> *ctx, TCGv arg, int reg, i
>              /* Stop translation as we may have switched the execution mode */
>              ctx->bstate = BS_STOP;
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 4:
>  //            gen_helper_mtc0_tracebpc(cpu_env, arg); /* PDtrace support */
>              /* Stop translation as we may have switched the execution mode */
>              ctx->bstate = BS_STOP;
>              rn = "TraceBPC";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -5150,30 +5167,37 @@ static void gen_mtc0 (CPUMIPSState *env, DisasContext 
> *ctx, TCGv arg, int reg, i
>  //            gen_helper_mtc0_performance1(arg);
>              rn = "Performance1";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 2:
>  //            gen_helper_mtc0_performance2(arg);
>              rn = "Performance2";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 3:
>  //            gen_helper_mtc0_performance3(arg);
>              rn = "Performance3";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 4:
>  //            gen_helper_mtc0_performance4(arg);
>              rn = "Performance4";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 5:
>  //            gen_helper_mtc0_performance5(arg);
>              rn = "Performance5";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 6:
>  //            gen_helper_mtc0_performance6(arg);
>              rn = "Performance6";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 7:
>  //            gen_helper_mtc0_performance7(arg);
>              rn = "Performance7";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -5417,6 +5441,7 @@ static void gen_dmfc0 (CPUMIPSState *env, DisasContext 
> *ctx, TCGv arg, int reg,
>  //            gen_helper_dmfc0_contextconfig(arg); /* SmartMIPS ASE */
>              rn = "ContextConfig";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -5690,18 +5715,22 @@ static void gen_dmfc0 (CPUMIPSState *env, 
> DisasContext *ctx, TCGv arg, int reg,
>  //            gen_helper_dmfc0_tracecontrol(arg, cpu_env); /* PDtrace 
> support */
>              rn = "TraceControl";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 2:
>  //            gen_helper_dmfc0_tracecontrol2(arg, cpu_env); /* PDtrace 
> support */
>              rn = "TraceControl2";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 3:
>  //            gen_helper_dmfc0_usertracedata(arg, cpu_env); /* PDtrace 
> support */
>              rn = "UserTraceData";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 4:
>  //            gen_helper_dmfc0_tracebpc(arg, cpu_env); /* PDtrace support */
>              rn = "TraceBPC";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -5727,30 +5756,37 @@ static void gen_dmfc0 (CPUMIPSState *env, 
> DisasContext *ctx, TCGv arg, int reg,
>  //            gen_helper_dmfc0_performance1(arg);
>              rn = "Performance1";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 2:
>  //            gen_helper_dmfc0_performance2(arg);
>              rn = "Performance2";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 3:
>  //            gen_helper_dmfc0_performance3(arg);
>              rn = "Performance3";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 4:
>  //            gen_helper_dmfc0_performance4(arg);
>              rn = "Performance4";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 5:
>  //            gen_helper_dmfc0_performance5(arg);
>              rn = "Performance5";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 6:
>  //            gen_helper_dmfc0_performance6(arg);
>              rn = "Performance6";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 7:
>  //            gen_helper_dmfc0_performance7(arg);
>              rn = "Performance7";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -5989,6 +6025,7 @@ static void gen_dmtc0 (CPUMIPSState *env, DisasContext 
> *ctx, TCGv arg, int reg,
>  //           gen_helper_mtc0_contextconfig(cpu_env, arg); /* SmartMIPS ASE */
>              rn = "ContextConfig";
>  //           break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -6274,24 +6311,28 @@ static void gen_dmtc0 (CPUMIPSState *env, 
> DisasContext *ctx, TCGv arg, int reg,
>              ctx->bstate = BS_STOP;
>              rn = "TraceControl";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 2:
>  //            gen_helper_mtc0_tracecontrol2(cpu_env, arg); /* PDtrace 
> support */
>              /* Stop translation as we may have switched the execution mode */
>              ctx->bstate = BS_STOP;
>              rn = "TraceControl2";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 3:
>  //            gen_helper_mtc0_usertracedata(cpu_env, arg); /* PDtrace 
> support */
>              /* Stop translation as we may have switched the execution mode */
>              ctx->bstate = BS_STOP;
>              rn = "UserTraceData";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 4:
>  //            gen_helper_mtc0_tracebpc(cpu_env, arg); /* PDtrace support */
>              /* Stop translation as we may have switched the execution mode */
>              ctx->bstate = BS_STOP;
>              rn = "TraceBPC";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -6317,30 +6358,37 @@ static void gen_dmtc0 (CPUMIPSState *env, 
> DisasContext *ctx, TCGv arg, int reg,
>  //            gen_helper_mtc0_performance1(cpu_env, arg);
>              rn = "Performance1";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 2:
>  //            gen_helper_mtc0_performance2(cpu_env, arg);
>              rn = "Performance2";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 3:
>  //            gen_helper_mtc0_performance3(cpu_env, arg);
>              rn = "Performance3";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 4:
>  //            gen_helper_mtc0_performance4(cpu_env, arg);
>              rn = "Performance4";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 5:
>  //            gen_helper_mtc0_performance5(cpu_env, arg);
>              rn = "Performance5";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 6:
>  //            gen_helper_mtc0_performance6(cpu_env, arg);
>              rn = "Performance6";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          case 7:
>  //            gen_helper_mtc0_performance7(cpu_env, arg);
>              rn = "Performance7";
>  //            break;
> +            /* XXX: questionable fallthrough */
>          default:
>              goto die;
>          }
> @@ -6506,6 +6554,7 @@ static void gen_mftr(CPUMIPSState *env, DisasContext 
> *ctx, int rt, int rd,
>                  gen_mfc0(env, ctx, t0, rt, sel);
>                  break;
>              }
> +            /* XXX: questionable fallthrough */
>          case 12:
>              switch (sel) {
>              case 0:
> @@ -6515,6 +6564,7 @@ static void gen_mftr(CPUMIPSState *env, DisasContext 
> *ctx, int rt, int rd,
>                  gen_mfc0(env, ctx, t0, rt, sel);
>                  break;
>              }
> +            /* XXX: questionable fallthrough */
>          case 13:
>              switch (sel) {
>              case 0:
> @@ -6724,6 +6774,7 @@ static void gen_mttr(CPUMIPSState *env, DisasContext 
> *ctx, int rd, int rt,
>                  gen_mtc0(env, ctx, t0, rd, sel);
>                  break;
>              }
> +            /* XXX: questionable fallthrough */
>          case 12:
>              switch (sel) {
>              case 0:
> @@ -6733,6 +6784,7 @@ static void gen_mttr(CPUMIPSState *env, DisasContext 
> *ctx, int rd, int rt,
>                  gen_mtc0(env, ctx, t0, rd, sel);
>                  break;
>              }
> +            /* XXX: questionable fallthrough */
>          case 13:
>              switch (sel) {
>              case 0:
> @@ -15382,6 +15434,7 @@ static void decode_opc (CPUMIPSState *env, 
> DisasContext *ctx, int *is_branch)
>              case OPC_MFHC1:
>              case OPC_MTHC1:
>                  check_insn(env, ctx, ISA_MIPS32R2);
> +                /* XXX: questionable fallthrough */
>              case OPC_MFC1:
>              case OPC_CFC1:
>              case OPC_MTC1:
> @@ -15515,6 +15568,7 @@ static void decode_opc (CPUMIPSState *env, 
> DisasContext *ctx, int *is_branch)
>      case OPC_MDMX:
>          check_insn(env, ctx, ASE_MDMX);
>          /* MDMX: Not implemented. */
> +        /* XXX: questionable fallthrough */
>      default:            /* Invalid */
>          MIPS_INVAL("major opcode");
>          generate_exception(ctx, EXCP_RI);

No idea.  Maybe our "odd fixer" Aurelien Jarno can help.

> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index 0aee7a9..2cf18fc 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -1740,6 +1740,7 @@ static int get_physical_address(CPUPPCState *env, 
> mmu_ctx_t *ctx,
>              if (env->nb_BATs != 0) {
>                  ret = get_bat(env, ctx, eaddr, rw, access_type);
>              }
> +            /* XXX: questionable fallthrough */
>  #if defined(TARGET_PPC64)
>          case POWERPC_MMU_620:
>          case POWERPC_MMU_64B:

Looks intentional to me.  Alex?

> diff --git a/target-s390x/translate.c b/target-s390x/translate.c
> index a57296c..be7c098 100644
> --- a/target-s390x/translate.c
> +++ b/target-s390x/translate.c
> @@ -3089,6 +3089,7 @@ static ExitStatus op_srnm(DisasContext *s, DisasOps *o)
>          break;
>      case 0xb9: /* SRNMT */
>          pos = 4, len = 3;
> +        /* XXX: questionable fallthrough */
>      default:
>          tcg_abort();
>      }

Suspicious.  Alex?

> diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
> index cf1bddf..2a8d86f 100644
> --- a/target-sparc/ldst_helper.c
> +++ b/target-sparc/ldst_helper.c
> @@ -1175,6 +1175,7 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
> addr, int asi, int size,
>          default:
>              break;
>          }
> +        /* XXX: questionable fallthrough */

Can just as well break here.

>      default:
>          break;
>      }
> @@ -1231,6 +1232,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong 
> addr, target_ulong val,
>          default:
>              break;
>          }
> +        /* XXX: questionable fallthrough */

Likewise.

>      default:
>          break;
>      }
> @@ -1615,6 +1617,7 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
> addr, int asi, int size,
>          default:
>              break;
>          }
> +        /* XXX: questionable fallthrough */

Likewise.

>      default:
>          break;
>      }
> @@ -1682,6 +1685,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong 
> addr, target_ulong val,
>          default:
>              break;
>          }
> +        /* XXX: questionable fallthrough */

Likewise.

>      default:
>          break;
>      }
> diff --git a/target-unicore32/translate.c b/target-unicore32/translate.c
> index f4498bc..b92437b 100644
> --- a/target-unicore32/translate.c
> +++ b/target-unicore32/translate.c
> @@ -1887,6 +1887,7 @@ static void disas_uc32_insn(CPUUniCore32State *env, 
> DisasContext *s)
>              do_misc(env, s, insn);
>              break;
>          }
> +        /* XXX: questionable fallthrough */
>      case 0x1:
>          if (((UCOP_OPCODES >> 2) == 2) && !UCOP_SET_S) {
>              do_misc(env, s, insn);
> @@ -1903,6 +1904,7 @@ static void disas_uc32_insn(CPUUniCore32State *env, 
> DisasContext *s)
>          if (UCOP_SET(8) || UCOP_SET(5)) {
>              ILLEGAL;
>          }
> +        /* XXX: questionable fallthrough */
>      case 0x3:
>          do_ldst_ir(env, s, insn);
>          break;

No idea.  Guan Xuetao?

> diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c
> index 3813a72..d829702 100644
> --- a/target-xtensa/op_helper.c
> +++ b/target-xtensa/op_helper.c
> @@ -443,8 +443,10 @@ void HELPER(check_atomctl)(CPUXtensaState *env, uint32_t 
> pc, uint32_t vaddr)
>      switch (access & PAGE_CACHE_MASK) {
>      case PAGE_CACHE_WB:
>          atomctl >>= 2;
> +        /* XXX: questionable fallthrough */
>      case PAGE_CACHE_WT:
>          atomctl >>= 2;
> +        /* XXX: questionable fallthrough */
>      case PAGE_CACHE_BYPASS:
>          if ((atomctl & 0x3) == 0) {
>              HELPER(exception_cause_vaddr)(env, pc,

Looks intentional.  Max Filippov?

> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index 973d2d6..bced966 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -635,6 +635,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s, 
> uint16_t *tcg_opc_ptr,
>              if ((temps[args[1]].mask & 0x80) != 0) {
>                  break;
>              }
> +            /* XXX: questionable fallthrough */
>          CASE_OP_32_64(ext8u):
>              mask = 0xff;
>              goto and_const;
> @@ -642,6 +643,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s, 
> uint16_t *tcg_opc_ptr,
>              if ((temps[args[1]].mask & 0x8000) != 0) {
>                  break;
>              }
> +            /* XXX: questionable fallthrough */
>          CASE_OP_32_64(ext16u):
>              mask = 0xffff;
>              goto and_const;
> @@ -649,6 +651,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s, 
> uint16_t *tcg_opc_ptr,
>              if ((temps[args[1]].mask & 0x80000000) != 0) {
>                  break;
>              }
> +            /* XXX: questionable fallthrough */
>          case INDEX_op_ext32u_i64:
>              mask = 0xffffffffU;
>              goto and_const;

No idea.

> diff --git a/ui/sdl.c b/ui/sdl.c
> index 1657848..aeceb7a 100644
> --- a/ui/sdl.c
> +++ b/ui/sdl.c
> @@ -552,6 +552,7 @@ static void handle_keydown(DisplayState *ds, SDL_Event 
> *ev)
>                  vga_hw_update();
>                  gui_keysym = 1;
>              }
> +            /* XXX: questionable fallthrough */

Can just as well break here.

>          default:
>              break;
>          }



reply via email to

[Prev in Thread] Current Thread [Next in Thread]