[PATCH hail] lib/hstor.c: avoid an unconditional leak in append_qparam

2010-09-27 Thread Jim Meyering
Signed-off-by: Jim Meyering meyer...@redhat.com --- I would have preferred to insert a single line right before the huri_field_escape call: char *v = strdup(val); [would result in a more compact, single-hunk patch] but it looks like hail uses the anachronistic (pre-C99) declare all vars at

Re: [PATCH hail] lib/hstor.c: avoid an unconditional leak in append_qparam

2010-09-27 Thread Pete Zaitcev
On Mon, 27 Sep 2010 10:53:06 +0200 Jim Meyering j...@meyering.net wrote: - stmp = huri_field_escape(strdup(val), QUERY_ESCAPE_MASK); + v = strdup(val); + stmp = huri_field_escape(v, QUERY_ESCAPE_MASK); str = g_string_append(str, stmp); free(stmp); + free(v); I

Re: [PATCH hail] lib/hstor.c: avoid an unconditional leak in append_qparam

2010-09-27 Thread Jeff Garzik
On 09/27/2010 04:53 AM, Jim Meyering wrote: Signed-off-by: Jim Meyeringmeyer...@redhat.com --- I would have preferred to insert a single line right before the huri_field_escape call: char *v = strdup(val); [would result in a more compact, single-hunk patch] but it looks like hail uses

Re: [PATCH hail] lib/hstor.c: avoid an unconditional leak in append_qparam

2010-09-27 Thread Jim Meyering
Pete Zaitcev wrote: On Mon, 27 Sep 2010 10:53:06 +0200 Jim Meyering j...@meyering.net wrote: -stmp = huri_field_escape(strdup(val), QUERY_ESCAPE_MASK); +v = strdup(val); +stmp = huri_field_escape(v, QUERY_ESCAPE_MASK); str = g_string_append(str, stmp); free(stmp); +

Re: [PATCH hail] lib/hstor.c: avoid an unconditional leak in append_qparam

2010-09-27 Thread Jeff Garzik
On 09/27/2010 12:29 PM, Pete Zaitcev wrote: On Mon, 27 Sep 2010 10:53:06 +0200 Jim Meyeringj...@meyering.net wrote: - stmp = huri_field_escape(strdup(val), QUERY_ESCAPE_MASK); + v = strdup(val); + stmp = huri_field_escape(v, QUERY_ESCAPE_MASK); str =

Re: [PATCH hail] lib/hstor.c: avoid an unconditional leak in append_qparam

2010-09-27 Thread Pete Zaitcev
On Mon, 27 Sep 2010 12:53:48 -0400 Jeff Garzik j...@garzik.org wrote: - stmp = huri_field_escape(strdup(val), QUERY_ESCAPE_MASK); + v = strdup(val); + stmp = huri_field_escape(v, QUERY_ESCAPE_MASK); str = g_string_append(str, stmp); free(stmp); + free(v); applied

Re: [hail patch 1/1] Fix calling convention of huri_field_escape

2010-09-27 Thread Jeff Garzik
On 09/27/2010 08:49 PM, Pete Zaitcev wrote: Premature optimization is the root of all evil. Use a sensible convention of not screwing with the argument, at the expense of extra strdup. Fortunately, all users are confined to Hail itself, even if huri_field_escape is exported. Signed-off-by:

Re: [tabled patch 1/1] Add a test for hstor_keys

2010-09-27 Thread Jeff Garzik
On 09/27/2010 08:52 PM, Pete Zaitcev wrote: Our current tests do not invoke hstor_keys at all, and so they did not catch a crash with double free in append_qparam. Add a very basic test which at least calls hstor_keys to verify that it does not crash right away. This test does not excercise