Re: [PATCH 1/9] hw/usb/redirect.c: Stop using qemu_oom_check()

[Eric Blake]

On Sat, Feb 26, 2022 at 06:07:15PM +0000, Peter Maydell wrote:
> 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.maydell@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);

Is exit(EXIT_FAILURE) worth using in this file?  We have an
inconsistent history of a magic number vs. a named constant, so either
way,

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org