Re: [evolution-patches] [calendar-plugins]: fix for bug #305627

["Carsten Guenther" <Carsten Guenther scalix com>]

Hello? Does anyone care? Can we revert that patch or at least check in 
Christian's changes?

Carsten Guenther wrote:

> I strongly feel that we should roll-back the original patch for the  
> following reasons:
>
> 1. questionable code quality (see Christian's list of issues)
> 2. this is a "fix" for a non-critical enhancement request during  
> feature-freeze
> 3. this patch not only introduces something Groupwise-specific (which  
> is a bad thing by itself) but also introduces a performance penalty  
> for all the other providers
>
> Carsten
>
> On Aug 17, 2005, at 8:18 PM, Christian Kellner wrote:
>
> > I was just looking through e-p patches and saw this patch and spotted
> > these things:
> >
> > 1)
> >
> > +        url = parent_object.url;
> > +        uri = camel_url_to_string (url, CAMEL_URL_HIDE_ALL);
> > +
> > +        groups = e_source_list_peek_groups (pitip-> ...
> >
> > camel_url_to_string () gives you a newly malloc'ed string you should
> > free, which you don't.
> >
> > 2)
> >
> > +         if (!strcmp (uri, e_source_get_uri (source))) {
> > +                    found = TRUE;
> > +                    break;
> > +         }
> >
> > e_source_get_uri () also gives you a newly malloc'ed uri which you
> > should free. Doing that in a loop is even worse.
> >
> > Some people out there are really trying hard to to spot old leaks  so we
> > should be very careful when writing new code, especially if we are so
> > close to a release.
> >
> > 3)
> > +                CamelService parent_object;
> > +                CamelURL *url;
> > [...]
> > +                parent_store = folder->parent_store;
> > +                parent_object = parent_store->parent_object;
> > +                url = parent_object.url;
> >
> > Really complicated code for for a simple dereference.
> >
> >
> > Attached is a patch to fix these issues.
> >
> > Yours,
> > Christian
> >
> > P.S.: On a side note, that doesn't really look like a bug fix for me,
> > but more as a speed enhancement for GW although we were in feature
> > freeze. Oh well.
> >
> > <itip.patch>
> > _______________________________________________
> > evolution-patches mailing list
> > evolution-patches lists ximian com
> > http://lists.ximian.com/mailman/listinfo/evolution-patches
>
> _______________________________________________
> evolution-patches mailing list
> evolution-patches lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-patches