Re: [evolution-patches] Bug 45210 - LDAP PostalAddress compliance patch for evolution-data-server, contacts

[Chris Toshok <toshok ximian com>]

On Mon, 2004-05-03 at 16:12 -0500, Edward Rudd wrote:
> On Mon, 2004-05-03 at 12:39, Chris Toshok wrote:
> > On Sun, 2004-05-02 at 13:37 -0500, Edward Rudd wrote:
> > > Here is an updated version of my patch to fix the storage postalAddress
> > > fields in LDAP.  This is based off of evolution-data-server CVS HEAD.
> > 
> > 
> > +struct berval **
> > +address_ber(EContact * card, EContactField field)
> > +{
> > +       struct berval **result = NULL;
> > +       const char *address;
> > +       char *temp, *i;
> > +
> > +       address = e_contact_get(card, field);
> > +       if (address) {
> > +               temp = g_strdup(address);
> > +               for (i = temp; *i != '\0'; i++) {
> > +                       if (*i == '\n') {
> > +                               *i = '$';
> > +                       }
> > +               }
> > +
> > +               result = g_new(struct berval *, 2);
> > +               result[0] = g_new(struct berval, 1);
> > +               result[0]->bv_val = temp;
> > +               result[0]->bv_len = strlen(temp);
> > +
> > +               result[1] = NULL;
> > +       }
> > +       return result;
> > +}
> > 
> > So this will leak the label - the memory you get back from e_contact_get
> > is to be freed anyway, so just removing temp and using address in its
> > place should fix things.
> > 
> Well, from tacking down what e_contact_get does, returns a gpointer
> retrieved from g_object_get, and not a copy of the structure.

e_contact_get on string fields (like the address labels) returns
allocated space for the string.  Trust me, I wrote the code.
e_contact_get_const is what you want to use if you don't want the copy,
but in this case you do want the copy - just not the extra g_strdup.

>  And all
> the other *_ber functions for complex type handling never touch the data
> retrieved from e_contact_get and type them as const char *, and they
> always make a copy which they place in the (berval*)->bv-val.  Therefore
> no leak is happening at all as I am putting the fixed temp var directly
> into the berval stucture. 

No, therefore the other *_ber functions (and lots of other functions in
there) are leaking :)  look at email_compare.  it uses (correctly)
e_contact_get_const.

Chris