Re: [Tracker] couple of fixes

[Jamie McCracken <jamiemcc blueyonder co uk>]

Samuel Cormier-Iijima wrote:
> On 9/21/06, Jamie McCracken <jamiemcc blueyonder co uk> wrote:
> > Jamie McCracken wrote:
> > > Samuel Cormier-Iijima wrote:
> > >> OK, I've split this up into two patches, and got rid of changing all
> > >> of the tracker_logs to g_messages. So now, tracker_log is just a macro
> > >> that expands to g_message. inotify.patch is the patch for numbers 1
> > >> and 3. Cheers,
> > >
> > > Okay the intoify patch is a bit tricky to review due to the complexity
> > > of the code. (I am a bit apprehensive about changing them as it can
> > > introduce new bugs and leaks)
> > >
> > > I think the delete_watch leaks res object in your patch
> > >
> > > I will look at it a bit more tomorrow but I may only accept part of 
> > your
> > > patch if thats okay with you.
> > >
> > > The inotify move stuff is especially  complex but well tested and so I
> > > want to keep that bit as is. I will accept the rest though. (I hope you
> > > understand the time taking to review *and* test for bugs and leaks can
> > > be excessive especially with time in short supply)
> >
> > sorry for my rant - just been short on time lately.
> >
> > I've now completed review of your entire inotify patch
> >
> > I've accepted it all but have made the following changes:
> >
> > 1) I put back the if (str) g_free (str) stuff at the end of
> > process_inotify_events which your patch removed.
> >
> > In general, its almost always better to have these then not to prevent a
> > sigabort from glibc shutting down tracker if we try and free stuff thats
> > not allocated.
> >
> > In process_inotify_events monitor_name can be NULL so its definitely
> > needed here.
>
> Actually, if str is NULL, g_free returns and does nothing:
> http://developer.gnome.org/doc/API/2.0/glib/glib-Memory-Allocation.html#g-free 
>
>
> So therfore, if (str) g_free (str) and g_free (str) are equivalent,
> and actually the first is also redundant and takes more code :-) But
> anyways that's a small point :-)

I agree but from experience I have used g_free on NULL before and got a 
sigabrt - better safe than sorry :)

--
Mr Jamie McCracken
http://jamiemcc.livejournal.com/