Re: [Qemu-devel] [PATCH] xlnx_dp: fix iffy xlnx_dp_aux_push_tx_fifo

[KONRAD Frederic]

Le 21/06/2016 à 15:16, Paolo Bonzini a écrit :
> xlnx_dp_aux_push_tx_fifo takes an immediate uint8_t and a buffer length,
> which must be 1 because that is how many uint8_t's fit in a uint8_t.
> Sure enough, that is what xlnx_dp_write passes to it, but the function
> is just weird.  Therefore, make xlnx_dp_aux_push_tx_fifo look like
> xlnx_dp_aux_push_rx_fifo, taking a pointer to the buffer.
>
> Reported by Coverity.

Hi Paolo,

Seems ok to me.

> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  hw/display/xlnx_dp.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index be53b75..f43eb09 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -438,10 +438,10 @@ static void xlnx_dp_aux_clear_tx_fifo(XlnxDPState *s)
>      fifo8_reset(&s->tx_fifo);
>  }
>
> -static void xlnx_dp_aux_push_tx_fifo(XlnxDPState *s, uint8_t val, size_t len)
> +static void xlnx_dp_aux_push_tx_fifo(XlnxDPState *s, uint8_t *buf, size_t len)
>  {
>      DPRINTF("Push %u data in tx_fifo\n", (unsigned)len);
> -    fifo8_push_all(&s->tx_fifo, &val, len);
> +    fifo8_push_all(&s->tx_fifo, buf, len);
>  }
>
>  static uint8_t xlnx_dp_aux_pop_tx_fifo(XlnxDPState *s)
> @@ -806,9 +806,11 @@ static void xlnx_dp_write(void *opaque, hwaddr offset, 
> uint64_t value,
>           * TODO: Power down things?
>           */
>          break;
> -    case DP_AUX_WRITE_FIFO:
> -        xlnx_dp_aux_push_tx_fifo(s, value, 1);
> +    case DP_AUX_WRITE_FIFO: {
> +        uint8_t c = value;
> +        xlnx_dp_aux_push_tx_fifo(s, &c, 1);
>          break;
> +    }

BTW do you need those braces here?

Thanks,
Fred

>      case DP_AUX_CLOCK_DIVIDER:
>          break;
>      case DP_AUX_REPLY_COUNT: