Re: [libvirt] [PATCH RFC 0/2] Report OOM on VIR_ALLOC failure
On Fri, Mar 22, 2013 at 12:44:55PM +0100, Michal Privoznik wrote:
Currently, our code is plenty of following scheme:
if (VIR_ALLOC(dummyPtr) < 0) {
virReportOOMError();
goto cleanup;
}
or something similar. What if we just move the OOM reporting into
VIR_ALLOC? It would have three nice features:
1) sizeof(code base) gets lower. A lot lower.
Yep, makes sense.
2) even for callers which don't follow the schema described
above, there is no harm reporting so serious error in the
logs. No matter that the callee may fall back and return
success.
I think I'd like some analysis of just how many places we
have which call VIR_ALLOC, but don't want to report errors.
Ought to be possible to grep it, or worst case a quick perl
script to analyse thing
At very least we need to be careful with virerror.c and
virlog.c, since both of those allocate memory, and we
don't want them to cause recursion here.
3) Removing virReportOOMError() from the schema does not need to
be done at once, but can be split into several patches. In the
worst case scenario - the error gets reported twice.
If we do this, we must convert everything before any release.
Overwriting errors is something to be avoided, so I don't
want us to make this worse.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|