Re: [Qemu-devel] [PULL 03/74] es1370: simplify MemoryRegionOps

[Peter Maydell]

On 21 August 2018 at 18:01, Paolo Bonzini <address@hidden> wrote:
> Use the automatic subregister extraction from the memory API, and avoid
> that Coverity complains about missing fallthrough comments.

Hi -- Coverity (CID 1395167) points out that these changes are wrong:

> @@ -572,21 +497,19 @@ static void es1370_writel(void *opaque, uint32_t addr, 
> uint32_t val)
>          print_sctl (val);
>          break;
>
> -    case ES1370_REG_ADC_SCOUNT:
> -        d++;
> -    case ES1370_REG_DAC2_SCOUNT:
> -        d++;
>      case ES1370_REG_DAC1_SCOUNT:
> +    case ES1370_REG_DAC2_SCOUNT:
> +    case ES1370_REG_ADC_SCOUNT:
> +        d += (addr - ES1370_REG_DAC1_SCOUNT) >> 2;
>          d->scount = (val & 0xffff) | (d->scount & ~0xffff);
>          ldebug ("chan %td CURR_SAMP_CT %d, SAMP_CT %d\n",
>                  d - &s->chan[0], val >> 16, (val & 0xffff));
>          break;
>
> -    case ES1370_REG_ADC_FRAMEADR:
> -        d++;
> -    case ES1370_REG_DAC2_FRAMEADR:
> -        d++;
>      case ES1370_REG_DAC1_FRAMEADR:
> +    case ES1370_REG_DAC2_FRAMEADR:
> +    case ES1370_REG_ADC_FRAMEADR:
> +        d += (addr - ES1370_REG_DAC1_FRAMEADR) >> 3;

because these values aren't contiguous:

#define ES1370_REG_DAC1_FRAMEADR    0xc30
#define ES1370_REG_DAC2_FRAMEADR    0xc38
#define ES1370_REG_ADC_FRAMEADR     0xd30

so you can't calculate the value of 'd' from the addr
this way.

(Similarly for the SCOUNT registers.)

thanks
-- PMM