Re: [PATCH 2/7] mac_via: fix up adb_via_receive() trace events

[Laurent Vivier]

Le 10/03/2021 à 09:09, Mark Cave-Ayland a écrit :
> The use of the post-increment operator on adb_data_in_index meant that the
> trace-event was accidentally displaying the next byte in the incoming ADB
> data buffer rather than the current byte.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/misc/mac_via.c | 41 ++++++++++++++++++++++++-----------------
>  1 file changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
> index 488d086a17..0f6586e102 100644
> --- a/hw/misc/mac_via.c
> +++ b/hw/misc/mac_via.c
> @@ -816,33 +816,37 @@ static void adb_via_receive(MacVIAState *s, int state, 
> uint8_t *data)
>          switch (s->adb_data_in_index) {
>          case 0:
>              /* First EVEN byte: vADBInt indicates bus timeout */
> -            trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
> -                                   *data, (ms->b & VIA1B_vADBInt) ? "+" : 
> "-",
> -                                   adb_bus->status, s->adb_data_in_index,
> -                                   s->adb_data_in_size);
> -
> -            *data = s->adb_data_in[s->adb_data_in_index++];
> +            *data = s->adb_data_in[s->adb_data_in_index];
>              if (adb_bus->status & ADB_STATUS_BUSTIMEOUT) {
>                  ms->b &= ~VIA1B_vADBInt;
>              } else {
>                  ms->b |= VIA1B_vADBInt;
>              }
> -            break;
>  
> -        case 1:
> -            /* First ODD byte: vADBInt indicates SRQ */
>              trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
>                                     *data, (ms->b & VIA1B_vADBInt) ? "+" : 
> "-",
>                                     adb_bus->status, s->adb_data_in_index,
>                                     s->adb_data_in_size);
>  
> -            *data = s->adb_data_in[s->adb_data_in_index++];
> +            s->adb_data_in_index++;
> +            break;
> +
> +        case 1:
> +            /* First ODD byte: vADBInt indicates SRQ */
> +            *data = s->adb_data_in[s->adb_data_in_index];
>              pending = adb_bus->pending & ~(1 << (s->adb_autopoll_cmd >> 4));
>              if (pending) {
>                  ms->b &= ~VIA1B_vADBInt;
>              } else {
>                  ms->b |= VIA1B_vADBInt;
>              }
> +
> +            trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
> +                                   *data, (ms->b & VIA1B_vADBInt) ? "+" : 
> "-",
> +                                   adb_bus->status, s->adb_data_in_index,
> +                                   s->adb_data_in_size);
> +
> +            s->adb_data_in_index++;
>              break;
>  
>          default:
> @@ -852,14 +856,9 @@ static void adb_via_receive(MacVIAState *s, int state, 
> uint8_t *data)
>               * end of the poll reply, so provide these extra bytes below to
>               * keep it happy
>               */
> -            trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
> -                                   *data, (ms->b & VIA1B_vADBInt) ? "+" : 
> "-",
> -                                   adb_bus->status, s->adb_data_in_index,
> -                                   s->adb_data_in_size);
> -
>              if (s->adb_data_in_index < s->adb_data_in_size) {
>                  /* Next data byte */
> -                *data = s->adb_data_in[s->adb_data_in_index++];
> +                *data = s->adb_data_in[s->adb_data_in_index];
>                  ms->b |= VIA1B_vADBInt;
>              } else if (s->adb_data_in_index == s->adb_data_in_size) {
>                  if (adb_bus->status & ADB_STATUS_BUSTIMEOUT) {
> @@ -869,7 +868,6 @@ static void adb_via_receive(MacVIAState *s, int state, 
> uint8_t *data)
>                      /* Return 0x0 after reply */
>                      *data = 0;
>                  }
> -                s->adb_data_in_index++;
>                  ms->b &= ~VIA1B_vADBInt;
>              } else {
>                  /* Bus timeout (no more data) */
> @@ -878,6 +876,15 @@ static void adb_via_receive(MacVIAState *s, int state, 
> uint8_t *data)
>                  adb_bus->status = 0;
>                  adb_autopoll_unblock(adb_bus);
>              }
> +
> +            trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
> +                                   *data, (ms->b & VIA1B_vADBInt) ? "+" : 
> "-",
> +                                   adb_bus->status, s->adb_data_in_index,
> +                                   s->adb_data_in_size);
> +
> +            if (s->adb_data_in_index <= s->adb_data_in_size) {
> +                s->adb_data_in_index++;
> +            }
>              break;
>          }
>  
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>