On Fri, Dec 20, 2019 at 03:29:42PM -0500, Cole Robinson wrote:
On 12/20/19 7:43 AM, Fabiano Fidêncio wrote:
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of abort()".
As libvirt decided to take the path to not report OOM and simply abort when it happens, let's get rid of the no_memory labels and simplify the code around them.
Series: Reviewed-by: Cole Robinson <crobinso@redhat.com>
The two exceptions are: - phyp code, as libvirt may end up dropping this code entirely; - virfirewall.c code, as it seems we heavily really on firewall->err being set to ENOMEM;
I looked at it a bit. It can probably all be ripped out but it's a little convoluted. virCommand seems to have some similar ENOMEM handling as well.
I think a nice prep step that will simplify this style of cleanups, is to drop the return value from the VIR_*LLOC* macros. After the glib conversion, they always return 0, or abort. But everywhere in the code is still checking for 'if (VIR_ALLOC(foo) < 0)' and similar.
Long term we should replace that with g_new0 but it's not a drop in replacement. An easy intermediate step we can do is entirely drop the '< 0' checking. This will removal a lot of 'if' conditionals that would need to be tweaked if we work on dropping cleanup: labels now. It could be mass done per directory and outside of a few cases I think they would all be trivial.
When we first introduced the use of glib we declared that we should *not* simply remove the '< 0' from VIR_ALLOC statements, because this will double the churn in the code base by making it a 2 step conversion. We want to go straight to g_new0, which is why we kept the "ATTRIBUTE_RETURN_CHECK" annotation on VIR_ALLOC & friends. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|