Clean up password prompting logic in streamutil.c.

The previous coding was fairly unreadable and drew double-free warnings
from clang.  I believe the double free was actually not reachable, because
PQconnectionNeedsPassword is coded to not return true if a password was
provided, so that the loop can't iterate more than twice.  Nonetheless
it seems worth rewriting.  No back-patch since this is just cosmetic.

diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 1dfb80f4a72a5e3d6e2fada9d24e25f8e6f93829..67917512e25cebca0ce26e0101dbb39d9cfea50d 100644 (file)
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -40,8 +40,8 @@ GetConnection(void)
    int         i;
    const char **keywords;
    const char **values;
-   char       *password = NULL;
    const char *tmpparam;
+   bool        need_password;
    PQconninfoOption *conn_opts = NULL;
    PQconninfoOption *conn_opt;
    char       *err_msg = NULL;
@@ -114,27 +114,30 @@ GetConnection(void)
        i++;
    }
 
+   /* If -W was given, force prompt for password, but only the first time */
+   need_password = (dbgetpassword == 1 && dbpassword == NULL);
+
    while (true)
    {
-       if (password)
-           free(password);
+       /* Get a new password if appropriate */
+       if (need_password)
+       {
+           if (dbpassword)
+               free(dbpassword);
+           dbpassword = simple_prompt(_("Password: "), 100, false);
+           need_password = false;
+       }
 
+       /* Use (or reuse, on a subsequent connection) password if we have it */
        if (dbpassword)
        {
-           /*
-            * We've saved a password when a previous connection succeeded,
-            * meaning this is the call for a second session to the same
-            * database, so just forcibly reuse that password.
-            */
            keywords[i] = "password";
            values[i] = dbpassword;
-           dbgetpassword = -1; /* Don't try again if this fails */
        }
-       else if (dbgetpassword == 1)
+       else
        {
-           password = simple_prompt(_("Password: "), 100, false);
-           keywords[i] = "password";
-           values[i] = password;
+           keywords[i] = NULL;
+           values[i] = NULL;
        }
 
        tmpconn = PQconnectdbParams(keywords, values, true);
@@ -150,63 +153,62 @@ GetConnection(void)
            exit(1);
        }
 
+       /* If we need a password and -w wasn't given, loop back and get one */
        if (PQstatus(tmpconn) == CONNECTION_BAD &&
            PQconnectionNeedsPassword(tmpconn) &&
            dbgetpassword != -1)
        {
-           dbgetpassword = 1;  /* ask for password next time */
-           PQfinish(tmpconn);
-           continue;
-       }
-
-       if (PQstatus(tmpconn) != CONNECTION_OK)
-       {
-           fprintf(stderr, _("%s: could not connect to server: %s\n"),
-                   progname, PQerrorMessage(tmpconn));
            PQfinish(tmpconn);
-           free(values);
-           free(keywords);
-           if (conn_opts)
-               PQconninfoFree(conn_opts);
-           return NULL;
+           need_password = true;
        }
+       else
+           break;
+   }
 
-       /* Connection ok! */
+   if (PQstatus(tmpconn) != CONNECTION_OK)
+   {
+       fprintf(stderr, _("%s: could not connect to server: %s\n"),
+               progname, PQerrorMessage(tmpconn));
+       PQfinish(tmpconn);
        free(values);
        free(keywords);
        if (conn_opts)
            PQconninfoFree(conn_opts);
+       return NULL;
+   }
 
-       /*
-        * Ensure we have the same value of integer timestamps as the server
-        * we are connecting to.
-        */
-       tmpparam = PQparameterStatus(tmpconn, "integer_datetimes");
-       if (!tmpparam)
-       {
-           fprintf(stderr,
-                   _("%s: could not determine server setting for integer_datetimes\n"),
-                   progname);
-           PQfinish(tmpconn);
-           exit(1);
-       }
+   /* Connection ok! */
+   free(values);
+   free(keywords);
+   if (conn_opts)
+       PQconninfoFree(conn_opts);
+
+   /*
+    * Ensure we have the same value of integer timestamps as the server we
+    * are connecting to.
+    */
+   tmpparam = PQparameterStatus(tmpconn, "integer_datetimes");
+   if (!tmpparam)
+   {
+       fprintf(stderr,
+        _("%s: could not determine server setting for integer_datetimes\n"),
+               progname);
+       PQfinish(tmpconn);
+       exit(1);
+   }
 
 #ifdef HAVE_INT64_TIMESTAMP
-       if (strcmp(tmpparam, "on") != 0)
+   if (strcmp(tmpparam, "on") != 0)
 #else
-       if (strcmp(tmpparam, "off") != 0)
+   if (strcmp(tmpparam, "off") != 0)
 #endif
-       {
-           fprintf(stderr,
+   {
+       fprintf(stderr,
             _("%s: integer_datetimes compile flag does not match server\n"),
-                   progname);
-           PQfinish(tmpconn);
-           exit(1);
-       }
-
-       /* Store the password for next run */
-       if (password)
-           dbpassword = password;
-       return tmpconn;
+               progname);
+       PQfinish(tmpconn);
+       exit(1);
    }
+
+   return tmpconn;
 }