Re: [PATCH 0/5] Get rid of "no_memory" labels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux