On 06/26/2018 03:03 PM, Peter Maydell wrote:
> We don't actually implement SD command CRC checking, because
> for almost all of our SD controllers the CRC generation is
> done in hardware, and so modelling CRC generation and checking
> would be a bit pointless. (The exception is that milkymist-memcard
> makes the guest software compute the CRC.)
>
> As a result almost all of our SD controller models don't bother
> to set the SDRequest crc field, and the SD card model doesn't
> check it. So the tracing of it in sdbus_do_command() provokes
> Coverity warnings about use of uninitialized data.
>
> Drop the CRC field from the trace; we can always add it back
> if and when we do anything useful with the CRC.
>
> Fixes Coverity issues 1386072, 1386074, 1386076, 1390571.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> We might also want to try to improve our CRC handling
> (eg so that the controller can say "I have not set the CRC,
> don't bother checking it" or "I have set the CRC, do check
> it because it's come from the guest software"), but let's
> put in the simple tweak to make Coverity happy for the moment.
Thanks for this simple patch... So simple I couldn't even think of it...
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> ---
> hw/sd/core.c | 2 +-
> hw/sd/trace-events | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sd/core.c b/hw/sd/core.c
> index 820345f704b..107e6d71ddb 100644
> --- a/hw/sd/core.c
> +++ b/hw/sd/core.c
> @@ -91,7 +91,7 @@ int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t
> *response)
> {
> SDState *card = get_card(sdbus);
>
> - trace_sdbus_command(sdbus_name(sdbus), req->cmd, req->arg, req->crc);
> + trace_sdbus_command(sdbus_name(sdbus), req->cmd, req->arg);
> if (card) {
> SDCardClass *sc = SD_CARD_GET_CLASS(card);
>
> diff --git a/hw/sd/trace-events b/hw/sd/trace-events
> index bfd1d62efcb..43cffab8b17 100644
> --- a/hw/sd/trace-events
> +++ b/hw/sd/trace-events
> @@ -7,7 +7,7 @@ bcm2835_sdhost_edm_change(const char *why, uint32_t edm)
> "(%s) EDM now 0x%x"
> bcm2835_sdhost_update_irq(uint32_t irq) "IRQ bits 0x%x\n"
>
> # hw/sd/core.c
> -sdbus_command(const char *bus_name, uint8_t cmd, uint32_t arg, uint8_t crc)
> "@%s CMD%02d arg 0x%08x crc 0x%02x"
> +sdbus_command(const char *bus_name, uint8_t cmd, uint32_t arg) "@%s CMD%02d
> arg 0x%08x"
> sdbus_read(const char *bus_name, uint8_t value) "@%s value 0x%02x"
> sdbus_write(const char *bus_name, uint8_t value) "@%s value 0x%02x"
> sdbus_set_voltage(const char *bus_name, uint16_t millivolts) "@%s %u (mV)"
>
