Re: [Tracker] couple of fixes

[Jamie McCracken <jamiemcc blueyonder co uk>]

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.

2) Added tracker_db_free_result (res) to delete_watch to fix the leak in 
your patch.


Have now added to cvs along with your logging patch - thanks for your 
time on this.


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