Re: [Libvir] [patch 1/5] iptables: fix invalid free
On Wed, Mar 21, 2007 at 01:30:00PM +0000, Mark McLoughlin wrote:
On Wed, 2007-03-21 at 09:03 -0400, Daniel Veillard wrote:
> On Wed, Mar 21, 2007 at 12:47:58PM +0000, Mark McLoughlin wrote:
> > In iptablesContextNew(), make sure we don't try and free an invalid
> > pointer if one of the iptRulesNew() fails.
> >
> > Signed-off-by: Mark McLoughlin <markmc(a)redhat.com>
> >
> > Index: libvirt/qemud/iptables.c
> > ===================================================================
> > --- libvirt.orig/qemud/iptables.c
> > +++ libvirt/qemud/iptables.c
> > @@ -496,7 +496,7 @@ iptablesContextNew(void)
> > {
> > iptablesContext *ctx;
> >
> > - if (!(ctx = (iptablesContext *) malloc(sizeof (iptablesContext))))
> > + if (!(ctx = (iptablesContext *) calloc(1, sizeof (iptablesContext))))
> > return NULL;
> >
> > if (!(ctx->input_filter = iptRulesNew("filter",
IPTABLES_PREFIX "INPUT")))
>
> I usually prefer malloc + memset( , 0, ) , but this probably comes from
> libxml2 where I replaced malloc calls with specific wrappers (and I still
> have a TODO for this in libvirt though some part of libvirt are not linked to
> libxml2 I guess so that may make things a bit harder)
If libvirt was going to have it's own malloc() wrapper, I'd suggest
that the wrapper should always zero out the allocated memory. Then we
could just replace the calloc() with the wrapper.
Well, one of the things I do in my wrapper is initilize to -1 all newly
allocated bytes, allows to pinpoint relatively easilly when you assumed zeroed
memory which was not, while not paying for the initialization when not needed.
Granted libvirt CPU usage can be considered neglectible compared to otehr parts
of the infrastructure.
Don't know why we'd use the libxml2 wrappers instead of our
own?
Because tehy exist, that libvirt uses libxml2, and that would integrate
the 2 memory reports
> What's the policy w.r.t. error reporting in qemud and
libvirt related daemons
> in general ? I guess a failure to malloc or thisd kind of problems should be
> logged somewhere, right ?
Yep. In this case it would be logged either to syslog if the daemon was
autostarting the network or returned to the user in the case where the
user is manually starting network.
okay, I assume those reporting layers are missing ATM, right ?
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/