On Sat, 26 Feb 2022 at 18:07, Peter Maydell <peter.mayd...@linaro.org> wrote:
Forgot to cc Gerd on this one as USB maintainer. Sorry..
> qemu_oom_check() is a function which essentially says "if you pass me
> a NULL pointer then print a message then abort()". On POSIX systems
> the message includes strerror(errno); on Windows it includes the
> GetLastError() error value printed as an integer.
>
> Other than in the implementation of qemu_memalign(), we use this
> function only in hw/usb/redirect.c, for three checks:
>
> * on a call to usbredirparser_create()
> * on a call to usberedirparser_serialize()
> * on a call to malloc()
>
> The usbredir library API functions make no guarantees that they will
> set errno on errors, let alone that they might set the
> Windows-specific GetLastError string. malloc() is documented as
> setting errno, not GetLastError -- and in any case the only thing it
> might set errno to is ENOMEM. So qemu_oom_check() isn't the right
> thing for any of these. Replace them with straightforward
> error-checking code. This will allow us to get rid of
> qemu_oom_check().
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> I have left all of these errors as fatal, since that's what they
> were previously. Possibly somebody with a better understanding
> of the usbredir code might be able to make them theoretically
> non-fatal, but we make malloc failures generally fatal anyway.
> ---
> hw/usb/redirect.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 5f0ef9cb3b0..8692ea25610 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -1239,7 +1239,11 @@ static void usbredir_create_parser(USBRedirDevice *dev)
>
> DPRINTF("creating usbredirparser\n");
>
> - dev->parser = qemu_oom_check(usbredirparser_create());
> + dev->parser = usbredirparser_create();
> + if (!dev->parser) {
> + error_report("usbredirparser_create() failed");
> + exit(1);
> + }
> dev->parser->priv = dev;
> dev->parser->log_func = usbredir_log;
> dev->parser->read_func = usbredir_read;
> @@ -2239,7 +2243,10 @@ static int usbredir_put_parser(QEMUFile *f, void
> *priv, size_t unused,
> }
>
> usbredirparser_serialize(dev->parser, &data, &len);
> - qemu_oom_check(data);
> + if (!data) {
> + error_report("usbredirparser_serialize failed");
> + exit(1);
> + }
>
> qemu_put_be32(f, len);
> qemu_put_buffer(f, data, len);
> @@ -2330,7 +2337,11 @@ static int usbredir_get_bufpq(QEMUFile *f, void *priv,
> size_t unused,
> bufp->len = qemu_get_be32(f);
> bufp->status = qemu_get_be32(f);
> bufp->offset = 0;
> - bufp->data = qemu_oom_check(malloc(bufp->len)); /* regular malloc! */
> + bufp->data = malloc(bufp->len); /* regular malloc! */
> + if (!bufp->data) {
> + error_report("usbredir_get_bufpq: out of memory");
> + exit(1);
> + }
> bufp->free_on_destroy = bufp->data;
> qemu_get_buffer(f, bufp->data, bufp->len);
> QTAILQ_INSERT_TAIL(&endp->bufpq, bufp, next);
> --
> 2.25.1
