Re: [Qemu-devel] [PATCH 2/6] rtl8139: g_malloc() can't fail, bury dead error handling

[Thomas Huth <thuth@linux.vnet.ibm.com>]

On Tue, 27 Jan 2015 17:38:27 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/net/rtl8139.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 6fa9e0a..b55e438 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -2075,20 +2075,6 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
>                  "length to %d\n", txsize);
>      }
> 
> -    if (!s->cplus_txbuffer)
> -    {
> -        /* out of memory */
> -
> -        DPRINTF("+++ C+ mode transmiter failed to reallocate %d bytes\n",
> -            s->cplus_txbuffer_len);
> -
> -        /* update tally counter */
> -        ++s->tally_counters.TxERR;
> -        ++s->tally_counters.TxAbt;
> -
> -        return 0;
> -    }
> -
>      /* append more data to the packet */
> 
>      DPRINTF("+++ C+ mode transmit reading %d bytes from host memory at "

Wouldn't it be better to use g_try_malloc() here instead? If the code
can handle OOM conditions, I think it's better to continue with a lost
packet here than to shut down QEMU the hard way.
(Also looking at the history of that file, the code originally used
qemu_malloc() which could fail - but instead of being replaced by
g_try_malloc(), it got replaced with g_malloc() instead which was
maybe the wrong decision).

 Thomas