Re: [GENERAL] Clang 3.3 Analyzer Results - Mailing list pgsql-hackers

From Magnus Hagander
Subject Re: [GENERAL] Clang 3.3 Analyzer Results
Date
Msg-id CABUevEyp6YgMHkrGBdN=TTFfwaHnYGxkBJwPVtiNsos++MZnKA@mail.gmail.com
Whole thread Raw
In response to Re: Clang 3.3 Analyzer Results  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [GENERAL] Clang 3.3 Analyzer Results  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Wednesday, November 13, 2013, Tom Lane wrote:
Kevin Grittner <kgrittn@ymail.com> writes:
> If nobody objects, I'll fix that small memory leak in the
> regression test driver.  Hopefully someone more familiar with
> pg_basebackup will fix the double-free (and related problems
> mentioned by Tom) in streamutil.c.

Here's a less convoluted (IMHO) approach to the password management logic
in streamutil.c.  One thing I really didn't care for about the existing
coding is that the loop-for-password included all the rest of the
function, even though there's no intention to retry for any purpose except
collecting a password.  So I moved up the bottom of the loop.  For ease of
review, I've not reindented the code below the new loop bottom, but would
do so before committing.

Any objections to this version?

Nope, looks good to me.

That code was originally "stolen" from psql, and then whacked around a number of times.  The part about looping and passwords, for example, is in startup.c in psql as well. We probably want to fix it there as well (even if it doesn't have the same problem, it has the same general design). Or perhaps even put that function somewhere shared between the two?

It's also in pg_dump/pg_backup_db.c, there's a version of it in pg_dumpall.c, etc. Which I think is a good argument for fixing them all by sharing the code somewhere? In fact, we already have some in script/common.c - but it's only used by the tools that are in script/.
 
//Magnus



--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

pgsql-hackers by date:

Previous
From: Sameer Thakur
Date:
Subject: Re: Extra functionality to createuser
Next
From: Magnus Hagander
Date:
Subject: Re: [PATCH 1/2] SSL: GUC option to prefer server cipher order