Re: [PATCH v2 7/9] hw/sd/sdcard: Do not allow invalid SD card sizes

[Peter Maydell]

On Mon, 13 Jul 2020 at 19:32, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> QEMU allows to create SD card with unrealistic sizes. This could
> work, but some guests (at least Linux) consider sizes that are not
> a power of 2 as a firmware bug and fix the card size to the next
> power of 2.
>

> +            error_setg(errp, "Invalid SD card size: %s", blk_size_str);
> +            g_free(blk_size_str);
> +
> +            blk_size_str = size_to_str(blk_size_aligned);
> +            error_append_hint(errp,
> +                              "SD card size has to be a power of 2, e.g. 
> %s.\n"
> +                              "You can resize disk images with "
> +                              "'qemu-img resize <imagefile> <new-size>'\n"
> +                              "(note that this will lose data if you make 
> the "
> +                              "image smaller than it currently is).\n",
> +                              blk_size_str);
> +            g_free(blk_size_str);

Some places that create multi-line hints with error_append_hint()
do it by calling it once per line, eg in target/arm/cpu64.c:
                error_setg(errp, "cannot disable sve128");
                error_append_hint(errp, "Disabling sve128 results in all "
                                  "vector lengths being disabled.\n");
                error_append_hint(errp, "With SVE enabled, at least one "
                                  "vector length must be enabled.\n");

Some places don't, eg in block/vhdx-log.c:
            error_append_hint(errp,  "To replay the log, run:\n"
                              "qemu-img check -r all '%s'\n",
                              bs->filename);

Most places terminate with a '\n', but some places don't, eg
crypto/block-luks.c:
       error_append_hint(errp, "Failed to write to keyslot %i", keyslot);
       return -1;

The documentation says
 * May be called multiple times.  The resulting hint should end with a
 * newline.

which isn't very clear -- you can call it multiple times, but
must you, if it's multiline?

I assume that "should end with a newline" means "must end
with a newline", and places like block-luks.c are bugs.

Markus, do you know what the intended API here is?

It looks like the implementation just tacks the hint
string onto the end of any existing hint string, in
which case multiple-line strings are fine and the same
behaviour as calling the function multiple times.
(I had assumed we might be accumulating an array of strings,
or requiring multiline strings to be multiple calls so we
could have the argument not need to be \n-terminated,
to match error_setg(), but both those assumptions
are obviously wrong.)

Anyway, I guess this multiline-message usage is something
we do already and it will DTRT, so

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>


thanks
-- PMM