Thomas Huth <th...@linux.vnet.ibm.com> writes: > On Tue, 27 Jan 2015 17:38:27 +0100 > Markus Armbruster <arm...@redhat.com> wrote: > >> Signed-off-by: Markus Armbruster <arm...@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).
When allocating 64KiB[*] fails, then you're a dead process walking. By checking for and recovering from such allocation failures, you can try to limp on for a bit. If your programmers have been perfectly diligent, you'll limp on until you die in a non-recoverable allocation failure. More likely, your programmers have been human, and you'll die an ugly death after some unchecked allocation or in some untested recovery that doesn't actually work. Our strategy is to use allocations that need checking only for large sizes and where recovery is possible without crippling loss of functionality. Those are rare. Gives us a chance to actually test their recovery. There's ample prededence for ditching untested allocation error recovery in favour of simply terminating the process. If you think recovery is worthwhile here, post a patch. Ideally extending tests/rtl8139-test.c to cover the error recovery. [*] That's the value of s->cplus_txbuffer_len (apparent once you peel off the obfuscation).