| Message ID | 19952e84d566d9309b15fe205db6b166f4234c33.1358697255.git.blauwirbel@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 20 January 2013 15:54, Blue Swirl <blauwirbel@gmail.com> wrote: This patch is a bit big to usefully review. A few comments on bits I happened to notice: > 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 */ Should have its own 'break' but it's safe currently as the following case is just 'break' anyway. > case 0x0c: /* OSC0 */ > case 0x10: /* OSC1 */ > case 0x14: /* OSC2 */ > @@ -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 */ Ditto. > case 0x54: /* CLCDSER */ > case 0x64: /* DMAPSR0 */ > case 0x68: /* DMAPSR1 */ > --- 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; These fallthroughs are clearly intentional (similar cases elsewhere in your patch). > --- 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 */ > case 0x4c: /* TBR */ > hw_error("TODO: Timer value read\n"); > + /* XXX: questionable fallthrough */ This isn't a fallthrough at all, hw_error() never returns. > default: > hw_error("gptm_read: Bad offset 0x%x\n", (int)offset); > return 0; (...so this return 0 is unreachable, but hey.) I don't think there's much point adding tons of "XXX" comments when a bunch of these aren't actually wrong code. If you want to fix this I think a better approach would be more focused patches aimed at adding 'break;' or "/* fallthrough */" based on actual human examination of the surrounding code. -- PMM
On Sun, Jan 20, 2013 at 4:56 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 20 January 2013 15:54, Blue Swirl <blauwirbel@gmail.com> wrote: > > This patch is a bit big to usefully review. A few comments on bits > I happened to notice: > >> 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 */ > > Should have its own 'break' but it's safe currently as the following > case is just 'break' anyway. > >> case 0x0c: /* OSC0 */ >> case 0x10: /* OSC1 */ >> case 0x14: /* OSC2 */ >> @@ -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 */ > > Ditto. > >> case 0x54: /* CLCDSER */ >> case 0x64: /* DMAPSR0 */ >> case 0x68: /* DMAPSR1 */ > >> --- 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; > > These fallthroughs are clearly intentional (similar cases > elsewhere in your patch). > >> --- 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 */ >> case 0x4c: /* TBR */ >> hw_error("TODO: Timer value read\n"); >> + /* XXX: questionable fallthrough */ > > This isn't a fallthrough at all, hw_error() never returns. > >> default: >> hw_error("gptm_read: Bad offset 0x%x\n", (int)offset); >> return 0; > > (...so this return 0 is unreachable, but hey.) > > I don't think there's much point adding tons of "XXX" comments > when a bunch of these aren't actually wrong code. If you want to fix > this I think a better approach would be more focused patches aimed > at adding 'break;' or "/* fallthrough */" based on actual human > examination of the surrounding code. The problem is that while some cases may be easy to decide, others are not so clear. My initial thought about the work flow was that this patch should be succeeded by other patches which replace the comment with correct action. These could be squashed to the original patch or committed later. If no decision can be made for some comment, it could stay as XXX. Alternatively, I could split this patch per maintainer, architecture or file even. Each maintainer could tune the patches as they see fit and commit whatever they want later. Probably some areas would be never fixed. > > -- PMM
Am 20.01.2013 18:26, schrieb Blue Swirl: > On Sun, Jan 20, 2013 at 4:56 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 20 January 2013 15:54, Blue Swirl <blauwirbel@gmail.com> wrote: >> >> This patch is a bit big to usefully review. A few comments on bits >> I happened to notice: [...] >>> --- 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 */ >>> case 0x4c: /* TBR */ >>> hw_error("TODO: Timer value read\n"); >>> + /* XXX: questionable fallthrough */ >> >> This isn't a fallthrough at all, hw_error() never returns. Maybe hw_error() needs some annotation instead? >> I don't think there's much point adding tons of "XXX" comments >> when a bunch of these aren't actually wrong code. If you want to fix >> this I think a better approach would be more focused patches aimed >> at adding 'break;' or "/* fallthrough */" based on actual human >> examination of the surrounding code. +1 > The problem is that while some cases may be easy to decide, others are > not so clear. > > My initial thought about the work flow was that this patch should be > succeeded by other patches which replace the comment with correct > action. These could be squashed to the original patch or committed > later. If no decision can be made for some comment, it could stay as > XXX. $ git grep XXX | wc --lines 75797 I don't think adding any more will help getting them addressed... > Alternatively, I could split this patch per maintainer, architecture > or file even. Each maintainer could tune the patches as they see fit > and commit whatever they want later. Probably some areas would be > never fixed. I would suggest to split per file and to propose either action rather than putting an XXX. I'm sure there would be static analysis volunteers to help review, CC'ing Stefan W. and Markus. :) Regards, Andreas
On 20 January 2013 17:38, Andreas Färber <afaerber@suse.de> wrote: > Am 20.01.2013 18:26, schrieb Blue Swirl: >> On Sun, Jan 20, 2013 at 4:56 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>>> case 0x4c: /* TBR */ >>>> hw_error("TODO: Timer value read\n"); >>>> + /* XXX: questionable fallthrough */ >>> >>> This isn't a fallthrough at all, hw_error() never returns. > > Maybe hw_error() needs some annotation instead? It is already marked QEMU_NORETURN. Presumably whatever tool Blue is using doesn't pay attention to noreturn annotations. -- PMM
> I don't think there's much point adding tons of "XXX" comments > when a bunch of these aren't actually wrong code. If you want to fix > this I think a better approach would be more focused patches aimed > at adding 'break;' or "/* fallthrough */" based on actual human > examination of the surrounding code. I agree. I encourage annotation of intentional fall through, but blindly pasting the output of an automated tool is liable to cause more harm than good. IMO running code analysis tools is easy. It's only when you take the time to manually inspect and fix the code that this really becomes valuable. Paul
Am 20.01.2013 16:54, schrieb Blue Swirl: > 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. > > Signed-off-by: Blue Swirl <blauwirbel@gmail.com> > 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); All IDE cases are clearly intentional. Kevin
Peter Maydell <peter.maydell@linaro.org> writes: > On 20 January 2013 15:54, Blue Swirl <blauwirbel@gmail.com> wrote: [...] > I don't think there's much point adding tons of "XXX" comments > when a bunch of these aren't actually wrong code. Moreover, such comments make them look intentional to static analyzers. I doubt lying to our tools is a good idea. > If you want to fix > this I think a better approach would be more focused patches aimed > at adding 'break;' or "/* fallthrough */" based on actual human > examination of the surrounding code. Indeed. I'd gladly provide a list of fall throughs Coverity dislikes. Additionally, I'd suggest to enforce a suitable convention for new code. I find this one sensible: either "break;" or "/* fall through */" is required, except right after a case label, a goto, continue, or return statement, or function call that never returns.
Blue Swirl <blauwirbel@gmail.com> 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 <blauwirbel@gmail.com> > --- > 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; > }
On Mon, Jan 21, 2013 at 5:11 PM, Markus Armbruster <armbru@redhat.com> wrote: > Blue Swirl <blauwirbel@gmail.com> 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 <blauwirbel@gmail.com> [...] >> 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? Correct, these are intentional.
Max Filippov <jcmvbkbc@gmail.com> writes: > On Mon, Jan 21, 2013 at 5:11 PM, Markus Armbruster <armbru@redhat.com> wrote: >> Blue Swirl <blauwirbel@gmail.com> 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 <blauwirbel@gmail.com> > > [...] > >>> 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? > > Correct, these are intentional. Thanks! Who's going to take care of adding /* fall through */ ?
On Mon, Jan 21, 2013 at 6:27 PM, Markus Armbruster <armbru@redhat.com> wrote: > Max Filippov <jcmvbkbc@gmail.com> writes: >>>> 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? >> >> Correct, these are intentional. > > Thanks! Who's going to take care of adding /* fall through */ ? I will post a patch, I just got wrong impression that you wanted them all fixed in one go.
> > diff --git a/disas/cris.c b/disas/cris.c > > + /* XXX: questionable fallthrough */ > > Inherited from binutils; if you want to clean this up, suggest to do it > there. Except that upstream binutils is GPLv3, so this code is effectively orphaned. Paul
On Mon, Jan 21, 2013 at 10:36 AM, Markus Armbruster <armbru@redhat.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: > >> On 20 January 2013 15:54, Blue Swirl <blauwirbel@gmail.com> wrote: > [...] >> I don't think there's much point adding tons of "XXX" comments >> when a bunch of these aren't actually wrong code. > > Moreover, such comments make them look intentional to static analyzers. > I doubt lying to our tools is a good idea. I see. That was not my intention. > >> If you want to fix >> this I think a better approach would be more focused patches aimed >> at adding 'break;' or "/* fallthrough */" based on actual human >> examination of the surrounding code. > > Indeed. I'd gladly provide a list of fall throughs Coverity dislikes. > > Additionally, I'd suggest to enforce a suitable convention for new code. > I find this one sensible: either "break;" or "/* fall through */" is > required, except right after a case label, a goto, continue, or return > statement, or function call that never returns. When Clang implements for example an attribute that can be used in C, we could add a Clang specific macro. Then all fall through comments could be replaced with this macro.
Blue Swirl <blauwirbel@gmail.com> writes: > On Mon, Jan 21, 2013 at 10:36 AM, Markus Armbruster <armbru@redhat.com> wrote: >> Peter Maydell <peter.maydell@linaro.org> writes: >> >>> On 20 January 2013 15:54, Blue Swirl <blauwirbel@gmail.com> wrote: >> [...] >>> I don't think there's much point adding tons of "XXX" comments >>> when a bunch of these aren't actually wrong code. >> >> Moreover, such comments make them look intentional to static analyzers. >> I doubt lying to our tools is a good idea. > > I see. That was not my intention. I figured that much :) >>> If you want to fix >>> this I think a better approach would be more focused patches aimed >>> at adding 'break;' or "/* fallthrough */" based on actual human >>> examination of the surrounding code. >> >> Indeed. I'd gladly provide a list of fall throughs Coverity dislikes. >> >> Additionally, I'd suggest to enforce a suitable convention for new code. >> I find this one sensible: either "break;" or "/* fall through */" is >> required, except right after a case label, a goto, continue, or return >> statement, or function call that never returns. > > When Clang implements for example an attribute that can be used in C, > we could add a Clang specific macro. Then all fall through comments > could be replaced with this macro. If I was working on Clang, I wouldn't bother. Instead, assume a fall through is intentional when there's a comment. Unlike attributes, the comments are already there, and other static checkers already work that way.
On Mon, Jan 21, 2013 at 9:10 PM, Markus Armbruster <armbru@redhat.com> wrote: > Blue Swirl <blauwirbel@gmail.com> writes: > >> On Mon, Jan 21, 2013 at 10:36 AM, Markus Armbruster <armbru@redhat.com> wrote: >>> Peter Maydell <peter.maydell@linaro.org> writes: >>> >>>> On 20 January 2013 15:54, Blue Swirl <blauwirbel@gmail.com> wrote: >>> [...] >>>> I don't think there's much point adding tons of "XXX" comments >>>> when a bunch of these aren't actually wrong code. >>> >>> Moreover, such comments make them look intentional to static analyzers. >>> I doubt lying to our tools is a good idea. >> >> I see. That was not my intention. > > I figured that much :) > >>>> If you want to fix >>>> this I think a better approach would be more focused patches aimed >>>> at adding 'break;' or "/* fallthrough */" based on actual human >>>> examination of the surrounding code. >>> >>> Indeed. I'd gladly provide a list of fall throughs Coverity dislikes. >>> >>> Additionally, I'd suggest to enforce a suitable convention for new code. >>> I find this one sensible: either "break;" or "/* fall through */" is >>> required, except right after a case label, a goto, continue, or return >>> statement, or function call that never returns. >> >> When Clang implements for example an attribute that can be used in C, >> we could add a Clang specific macro. Then all fall through comments >> could be replaced with this macro. > > If I was working on Clang, I wouldn't bother. Instead, assume a fall > through is intentional when there's a comment. Unlike attributes, the > comments are already there, and other static checkers already work that > way. At least __fallthrough and __builtin_fallthrough() were mentioned here: http://clang-developers.42468.n3.nabble.com/should-Wimplicit-fallthrough-require-C-11-td4028144.html
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; 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: ; } 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': 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; 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 */ case 0x0c: /* OSC0 */ case 0x10: /* OSC1 */ case 0x14: /* OSC2 */ @@ -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 */ case 0x54: /* CLCDSER */ case 0x64: /* DMAPSR0 */ case 0x68: /* DMAPSR1 */ 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: 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 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; 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; 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; 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); 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); 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; 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; 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 */ 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: 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", 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; 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; 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: 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); } 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 */ 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 */ case 0x4c: /* TBR */ hw_error("TODO: Timer value read\n"); + /* XXX: questionable fallthrough */ 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; 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; 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; @@ -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) 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; } 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) 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: 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); 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: 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(); } 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 */ default: break; } @@ -1231,6 +1232,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val, default: break; } + /* XXX: questionable fallthrough */ 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 */ default: break; } @@ -1682,6 +1685,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val, default: break; } + /* XXX: questionable fallthrough */ 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; 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, 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; 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 */ default: break; }
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. Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- 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(-)