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@xxxxxxxxxx> > > > 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
