Re: [Qemu-devel] [PATCH v2] scsi: esp: check TI buffer index before read/write

[Peter Maydell]

On 6 June 2016 at 13:00, Paolo Bonzini <address@hidden> wrote:
>
>
> On 31/05/2016 13:36, P J P wrote:
>> +        if ((s->rregs[ESP_RSTAT] & STAT_PIO_MASK) == 0) {
>> +            /* Data out.  */
>> +            qemu_log_mask(LOG_UNIMP, "esp: PIO data read not 
>> implemented\n");
>> +            s->rregs[ESP_FIFO] = 0;
>> +            esp_raise_irq(s);
>> +        } else if (s->ti_rptr < s->ti_wptr) {
>>              s->ti_size--;
>> -            if ((s->rregs[ESP_RSTAT] & STAT_PIO_MASK) == 0) {
>> -                /* Data out.  */
>> -                qemu_log_mask(LOG_UNIMP,
>> -                              "esp: PIO data read not implemented\n");
>> -                s->rregs[ESP_FIFO] = 0;
>> -            } else {
>> -                s->rregs[ESP_FIFO] = s->ti_buf[s->ti_rptr++];
>> -            }
>> +            s->rregs[ESP_FIFO] = s->ti_buf[s->ti_rptr++];
>>              esp_raise_irq(s);
>>          }
>>          if (s->ti_size == 0) {
>
> Shouldn't this become a "s->ti_rptr == s->ti_wptr" too?

It would probably be helpful to document what the intended invariant
relating ti_rptr, ti_wptr and ti_size is as well.

thanks
-- PMM