Hmm. It looks like my initial read may not have been entirely correct. I
think I've got it now.

Daniel Stenberg wrote:
>
> How exactly is the ->lud_attrs pointer freed twice? With you assigning
> it to NULL everywhere on errors you instead introduce memory leaks since
> it'll skip freeing completely. Or am I reading it wrong?
>

After reading this again, I don't think ->lud_attrs is freed twice.
Rather, the elements of ->lud_attrs are freed when they shouldn't be if
the function exits with an error.

> Bonus issue: unescape_elements() goes through several pointers and
> replaces them with "unescaped" versions. The only problem there is that
> curl_easy_unescape() returns a newly allocated string and the function
> doesn't free the previous strings that were unescaped and no longer
> used. It so looks like a memory leak to me!
>

Yes. This made it harder to read for me. Upon success
_ldap_free_urldesc() was properly freeing elements of ->lud_attrs that
had been allocated by unescape_elements. Upon error, those elements had
been populated by strtok_r and didn't need to be freed but were being
freed anyway.

So I think the right fix is to free the array but not the individual
elements whenever we're returning an error code prior to the
unescape_elements() call.

Does that match your read? If so I'll rewrite the patch.

Thanks,

Geoff
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2013-08-22