Re: [GENERAL] Clang 3.3 Analyzer Results

On Wednesday, November 13, 2013, Tom Lane wrote:

> Kevin Grittner <kgrittn(at)ymail(dot)com <javascript:;>> 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/