Re: [Qemu-devel] [PATCH] fix "Missing break in switch" coverity reports

[Peter Maydell]

On 1 August 2018 at 16:14, Paolo Bonzini <address@hidden> wrote:
> Many of these are marked as "intentional/fix required" because they
> just need adding a fall through comment.  This is exactly what this
> patch does, except for target/mips/translate.c where it is easier to
> duplicate the code, and hw/audio/sb16.c where I consulted the DOSBox
> sources and decide to just remove the LOG_UNIMP before the fallthrough.
>
> Signed-off-by: Paolo Bonzini <address@hidden>

> diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
> index 5a4d32364e..665a7e75c5 100644
> --- a/hw/audio/sb16.c
> +++ b/hw/audio/sb16.c
> @@ -741,10 +741,8 @@ static void complete (SB16State *s)
>              ldebug ("set time const %d\n", s->time_const);
>              break;
>
> -        case 0x42:              /* FT2 sets output freq with this, go figure 
> */
> -            qemu_log_mask(LOG_UNIMP, "cmd 0x42 might not do what it think it"
> -                          " should\n");
>          case 0x41:
> +        case 0x42:              /* FT2 sets output freq with this, go figure 
> */
>              s->freq = dsp_get_hilo (s);
>              ldebug ("set freq %d\n", s->freq);
>              break;

I agree with the outcome; see previous discussions
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02081.html
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02079.html

and this description of what the hardware does:
http://homepages.cae.wisc.edu/~brodskye/sb16doc/sb16doc.html#SamplingRate

We could improve the somewhat cryptic comment:
    /*
     * 0x41 is documented as setting the output sample rate,
     * and 0x42 the input sample rate, but in fact SB16 hardware
     * seems to have only a single sample rate under the hood,
     * and some (buggy) guest programs such as FT2 will set the
     * output rate using 0x42. Compare:
     * http://homepages.cae.wisc.edu/~brodskye/sb16doc/sb16doc.html#SamplingRate
     */

thanks
-- PMM