Re: [PULL 08/16] hw/sd/sdcard: Add sd_cmd_GEN_CMD handler (CMD56)

[Peter Maydell]

On Fri, 5 Jul 2024 at 23:06, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> "General command" (GEN_CMD, CMD56) is described as:
>
>   GEN_CMD is the same as the single block read or write
>   commands (CMD24 or CMD17). The difference is that [...]
>   the data block is not a memory payload data but has a
>   vendor specific format and meaning.
>
> Thus this block must not be stored overwriting data block
> on underlying storage drive. Handle as RAZ/WI.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
> Tested-by: Cédric Le Goater <clg@redhat.com>
> Message-Id: <20240703134356.85972-3-philmd@linaro.org>


> @@ -2187,11 +2183,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
>          break;
>
>      case 56:  /* CMD56:  GEN_CMD */
> -        sd->data[sd->data_offset ++] = value;
> -        if (sd->data_offset >= sd->blk_len) {
> -            APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
> -            sd->state = sd_transfer_state;
> -        }
> +        sd_generic_write_byte(sd, value);
>          break;
>
>      default:

Hi; Coverity notes that we almost always check the return
value from sd_generic_write_byte(), but that we don't do
that in this new callsite. Is there something we should do
if it returns false here, or should we just mark the
issue in coverity as a false positive? (CID 1550397)

thanks
-- PMM