[patch] fix-double-free patch (was Re: [patch] null after free)

[John (J5) Palmieri]

I went through the code and it looks like we are doing the right thing
everywhere else so I just removed the free in fill_user_info.  Here is
the patch.  Ok to commit?

On Tue, 2004-10-05 at 10:29 -0400, Havoc Pennington wrote:
> On Mon, 2004-10-04 at 16:28 -0400, John (J5) Palmieri wrote:
> > What is actually happening is that on an error in the fill_user_info
> > function in dbus-sysdeps.c we are jumping to the failed section which
> > frees the DBusUserInfo structure:
> > 
> > failed:
> >   _DBUS_ASSERT_ERROR_IS_SET (error);
> >   _dbus_user_info_free (info);
> >   return FALSE;
> 
> Ah, this is just plain old crackrock; should not free info there.
> 
> > However this returns FALSE and an error to the caller which then calls
> > free_user_info (which calls _dbus_user_info_free somewhere down the
> > line).  Here is an example from dbus-userdb.c
> > (_dbus_user_database_lookup):
> > 
> > if (!_dbus_user_info_fill_uid (info, uid, error))
> >   {
> >     _DBUS_ASSERT_ERROR_IS_SET (error);
> >     free_user_info (info);
> >     return NULL;
> >   }
> > 
> > This is just one example but it happens a couple of times.  The question
> > is where is it best to free the structure?  The DBusUserInfo field is
> > passed into fill_user_info so I feel this is not the place it should be
> > freed.  However if it is not freed there we must make sure the error is
> > handled everywhere else.  Any opinions?
> > 
> 
> Yes, the error should be handled and freed everywhere else. The same
> code that allocs should free.
> 
> Havoc
> 
> 
-- 
John (J5) Palmieri
Associate Software Engineer
Desktop Group
Red Hat, Inc.
Blog: http://martianrock.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dbus-fix-double-free.patch
Type: text/x-patch
Size: 1131 bytes
Desc: not available
Url : http://freedesktop.org/pipermail/dbus/attachments/20041005/b03e78ea/dbus-fix-double-free.bin