[collectd] Bug in utils_cache.c

[Sebastian Harl]

Hi,

On Wed, Jul 08, 2009 at 10:55:11AM +0200, Florian Forster wrote:
> On Tue, Jul 07, 2009 at 01:16:48PM -0600, Paul Sadauskas wrote:
> > tokkee on IRC & I think we found a bug with utils_cache.c. The
> > uc_check_timeout function is missing a continue after the
> > "uninteresting" service check, that causes a key to be null.
> 
> thanks for the patch. I think there should be a `continue' there, but
> the assertion failure may have resulted from a different problem: As you
> can see in the context of your patch, `ce' is freed in this case. But
> it's not allocated/written to in that loop before that, so it may free
> the entry of the previous iteration (without calling avl_remove). I
> think *that* caused the problem.

As far as I can see, that should be fine. 'ce' should be written to in
c_avl_remove(). However, there might be a problem if that fails (i.e.
status != 0) - imho, in that case we need a 'continue' as well or better
check for 'ce' not being NULL (and setting it to NULL before calling
c_avl_remove()). In fact, I think that commit 984f37c introduced a
memory leak by removing the call to cache_free(). If I did not miss
anything, the same holds true for the end of the loop as well, i.e. a
call to cache_free() is missing there as well.

Imho, the assertion was triggered by sfree(keys[i]), which sets keys[i]
to NULL which was then passed on to c_avl_get() later on.

Cheers,
Sebastian

-- 
Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/

Those who would give up Essential Liberty to purchase a little Temporary
Safety, deserve neither Liberty nor Safety.         -- Benjamin Franklin

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20090708/09541b6a/attachment-0001.pgp