bug-wget
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bug-wget] Memory leak in idn_encode; Valgrind suppression file


From: Hubert Tarasiuk
Subject: Re: [Bug-wget] Memory leak in idn_encode; Valgrind suppression file
Date: Mon, 06 Apr 2015 18:58:47 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

W dniu 06.04.2015 o 17:24, Ander Juaristi pisze:
> But just being pedantic, the reason of the leak is not because *new
> becomes out of scope, since idn_encode()
> is only called at url_parse(), and the *new pointer returned by the
> former is catched by the latter and stored in
> u->host, which is in the end freed at url_free() (kinda looks like, at a
> glance :D).

There is the confusion, because the same pointer (`new`) is used for two
separate memory locations in this function. (That's why I decided to add
another ptr variable in my patch.)

Since it is not clear, I will go over the code and explain:
> char *
> idn_encode (struct iri *i, char *host)
> {
>   char *new;
>   int ret;
> 
>   /* Encode to UTF-8 if not done */
>   if (!i->utf8_encode)
>     {
>       if (!remote_to_utf8 (i, (const char *) host, (const char **) &new))
>           return NULL;  /* Nothing to encode or an error occured */
At this point, remote_to_utf8 allocates a new buffer and stores its
result (encoded host name) in it. It sets `new` to point to that buffer.
>       host = new;
At this point, both `new` and `host` point to the result of remote_to_utf8.
>     }
> 
>   /* toASCII UTF-8 NULL terminated string */
>   ret = idna_to_ascii_8z (host, &new, IDNA_FLAGS);
At this point, `idn_to_ascii_8z` stores its result into a newly
allocated buffer and sets `new` to point to that buffer. (So the result
of `remote_to_utf8` is only available under `host`.)
>   if (ret != IDNA_SUCCESS)
>     {
>       /* sXXXav : free new when needed ! */
>       logprintf (LOG_VERBOSE, _("idn_encode failed (%d): %s\n"), ret,
>                  quote (idna_strerror (ret)));
>       return NULL;
>     }
> 
>   return new;
> }
The local variable `host` is not used ever again and goes away from
stack at this moment. The result of remote_to_utf8 is not freed and not
accessible anymore.

Do you see my point after the explanation?

Regards,
Hubert

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]