On Wed, 2013-11-13 at 12:43 -0500, Tom Lane wrote:
> Kevin Grittner <kgri...@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?
As far as the clang static analyzer is concerned, this hasn't actually
helped, because now it's complaining about
Value stored to 'need_password' is never read
With the attached patch, that warning goes way, and the logic is
arguably slightly clearer, too.
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 6791751..6cc6cd2 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -117,7 +117,7 @@
/* If -W was given, force prompt for password, but only the first time */
need_password = (dbgetpassword == 1 && dbpassword == NULL);
- while (true)
+ do
{
/* Get a new password if appropriate */
if (need_password)
@@ -161,9 +161,8 @@
PQfinish(tmpconn);
need_password = true;
}
- else
- break;
}
+ while (need_password);
if (PQstatus(tmpconn) != CONNECTION_OK)
{
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
