From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: Re: [PATCH hail] lib/hstor.c: avoid an unconditional leak in append_qparam Date: Mon, 27 Sep 2010 12:53:48 -0400 Message-ID: <4CA0CC1C.8040407@garzik.org> References: <87tylbeff1.fsf@meyering.net> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=cHw3jejsBUTCww4/GU0vI+vggFzsc4zE9vpByIefqVA=; b=G4uBfK+CrzTAF8QgUW7gh3D3xgaWw1izwlFRe4rlnUArLbjDbC9aFj16FUHtAWkXEZ A2DVvHVqIC5J6wrFtTJStCUUZ1MnTxUJGiu9YVNln18pP1uWswFL60DOhjFbUZw0+zTM I1P39Bq6aY+hApgjs6Qe+/bOdeRZWd/21PoQg= In-Reply-To: <87tylbeff1.fsf@meyering.net> Sender: hail-devel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii"; format="flowed" To: Jim Meyering Cc: Project Hail On 09/27/2010 04:53 AM, Jim Meyering wrote: > > > Signed-off-by: Jim Meyering > --- > 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 outer scope" style, so I conformed. > > lib/hstor.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/lib/hstor.c b/lib/hstor.c > index 6c67bfa..79e0420 100644 > --- a/lib/hstor.c > +++ b/lib/hstor.c > @@ -676,6 +676,7 @@ static GString *append_qparam(GString *str, const char *key, const char *val, > char *arg_char) > { > char *stmp; > + char *v; > > str = g_string_append(str, arg_char); > arg_char[0] = '&'; > @@ -683,9 +684,11 @@ static GString *append_qparam(GString *str, const char *key, const char *val, > str = g_string_append(str, key); > str = g_string_append(str, "="); > > - 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 Yeah, I don't like C++ var decls; I think the code gets too disorganized, making it really easy to miss a decl when reviewing. Jeff