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

2015-04-11 Thread Tim Rühsen
Am Donnerstag, 9. April 2015, 23:14:22 schrieb Hubert Tarasiuk: W dniu 08.04.2015 o 22:21, Tim Rühsen pisze: Could you use $srcdir instead Cwd ? I didn't test it, but it could fail when using DISTCHECK_CONFIGURE_FLAGS=VALGRIND_TESTS=1 make distcheck. It least we should use the same path

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

2015-04-11 Thread Hubert Tarasiuk
W dniu 10.04.2015 o 22:27, Tim Rühsen pisze: Hmmm, if the bug in idna_to_ascii_8z() has been fixed (as Hubert says), we should simply remove the comment (as Hubert suggests). Hubert, could you amend the patch ? Preferably with Ángel's variable renaming. Attached. Regards, Hubert From

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

2015-04-10 Thread Tim Rühsen
Am Mittwoch, 8. April 2015, 21:09:28 schrieb Ángel González: On 08/04/15 19:08, Hubert Tarasiuk wrote: W dniu 07.04.2015 o 00:11, Ángel González pisze: On 06/04/15 22:15, Hubert Tarasiuk wrote: We should probably also fix this comment: /* sXXXav : free new when needed ! */

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

2015-04-09 Thread Hubert Tarasiuk
W dniu 08.04.2015 o 22:21, Tim Rühsen pisze: Could you use $srcdir instead Cwd ? I didn't test it, but it could fail when using DISTCHECK_CONFIGURE_FLAGS=VALGRIND_TESTS=1 make distcheck. It least we should use the same path mechanism as with e.g. 'certs' in Test-proxied-https-auth.px. It

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

2015-04-08 Thread Hubert Tarasiuk
W dniu 07.04.2015 o 00:11, Ángel González pisze: On 06/04/15 22:15, Hubert Tarasiuk wrote: We should probably also fix this comment: /* sXXXav : free new when needed ! */ As it presumably mentions the problem that we are going to repair. I thought it refered to idna_to_ascii_8z

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

2015-04-08 Thread Ángel González
On 08/04/15 19:08, Hubert Tarasiuk wrote: W dniu 07.04.2015 o 00:11, Ángel González pisze: On 06/04/15 22:15, Hubert Tarasiuk wrote: We should probably also fix this comment: /* sXXXav : free new when needed ! */ As it presumably mentions the problem that we are going to repair. I

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

2015-04-08 Thread Hubert Tarasiuk
W dniu 06.04.2015 o 22:26, Tim Rühsen pisze: Valgrind suppressions are a bit compiler/architecture/distribution dependent. Maybe you could you add a comment into the suppression file with these infos. As a quick reference and explanation. I have added a reference URL to Redhat's bugzilla,

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

2015-04-08 Thread Hubert Tarasiuk
I forgot to add Makefile.am to commit message. Please find the updated patch attached, W dniu 08.04.2015 o 18:55, Hubert Tarasiuk pisze: W dniu 06.04.2015 o 22:26, Tim Rühsen pisze: Valgrind suppressions are a bit compiler/architecture/distribution dependent. Maybe you could you add a

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

2015-04-07 Thread Tim Rühsen
Am Montag, 6. April 2015, 22:26:27 schrieb Tim Rühsen: Hi Hubert, Am Montag, 6. April 2015, 12:57:18 schrieb Hubert Tarasiuk: Hello devs, Also, IMHO it is worth to add a suppression file for valgrind tests in wget. Otherwise, the tests do not pass. (Apart from the bug mentioned

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

2015-04-06 Thread Ángel González
Thanks for your work, Hubert. I agree it is helpful to name them differently. What about this alternative rename? (I'm the host reassignation by using a ternary, plus touched a comment) diff --git a/src/iri.c b/src/iri.c index dc1ebd0..eec5646 100644 --- a/src/iri.c +++ b/src/iri.c @@

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

2015-04-06 Thread Hubert Tarasiuk
W dniu 06.04.2015 o 21:25, Ángel González pisze: I agree it is helpful to name them differently. What about this alternative rename? It has the advantage of unconditional `xfree`, but the call to `idna_to_ascii_8z` is more complicated. It is hard to tell which one is better. (I'm the host

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

2015-04-06 Thread Ander Juaristi
On 04/06/2015 06:58 PM, Hubert Tarasiuk wrote: Do you see my point after the explanation? Sure, thanks. I didn't notice the fact that new was copied into host, my efforts were focusing on the flow of the variable new itself. -- Regards, - AJ

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

2015-04-06 Thread Tim Rühsen
Hi Hubert, Am Montag, 6. April 2015, 12:57:18 schrieb Hubert Tarasiuk: Hello devs, Also, IMHO it is worth to add a suppression file for valgrind tests in wget. Otherwise, the tests do not pass. (Apart from the bug mentioned above, there is a Valgrind's false positive at `idna_to_ascii_4z`

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

2015-04-06 Thread Ángel González
On 06/04/15 22:15, Hubert Tarasiuk wrote: We should probably also fix this comment: /* sXXXav : free new when needed ! */ As it presumably mentions the problem that we are going to repair. I thought it refered to idna_to_ascii_8z sometimes allocating memory on error (that's why I

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

2015-04-06 Thread Ander Juaristi
On 04/06/2015 12:57 PM, Hubert Tarasiuk wrote: The problem was, that `remote_to_utf8` would allocate a new buffer, but the buffer was never freed. (It was the `new` pointer, later copied to `host`, used for `idna_to_ascii_8z`. After returning from idn_encode, it was out of scope.) I'm not

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

2015-04-06 Thread Hubert Tarasiuk
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