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