8000 Simplify correct use of simple_prompt(). · postgres/postgres@9daec77 · GitHub
[go: up one dir, main page]
Skip to content

Commit 9daec77

Browse files
tglsfdctglsfdc
committed
Simplify correct use of simple_prompt().
The previous API for this function had it returning a malloc'd string. That meant that callers had to check for NULL return, which few of them were doing, and it also meant that callers had to remember to free() the string later, which required extra logic in most cases. Instead, make simple_prompt() write into a buffer supplied by the caller. Anywhere that the maximum required input length is reasonably small, which is almost all of the callers, we can just use a local or static array as the buffer instead of dealing with malloc/free. A fair number of callers used "pointer == NULL" as a proxy for "haven't requested the password yet". Maintaining the same behavior requires adding a separate boolean flag for that, which adds back some of the complexity we save by removing free()s. Nonetheless, this nets out at a small reduction in overall code size, and considerably less code than we would have had if we'd added the missing NULL-return checks everywhere they were needed. In passing, clean up the API comment for simple_prompt() and get rid of a very-unnecessary malloc/free in its Windows code path. This is nominally a bug fix, but it does not seem worth back-patching, because the actual risk of an OOM failure in any of these places seems pretty tiny, and all of them are client-side not server-side anyway. This patch is by me, but it owes a great deal to Michael Paquier who identified the problem and drafted a patch for fixing it the other way. Discussion: <CAB7nPqRu07Ot6iht9i9KRfYLpDaF2ZuUv5y_+72uP23ZAGysRg@mail.gmail.com>
1 parent 37f6fd1 commit 9daec77

File tree

15 files changed

+138
-142
lines changed
  • pg_dumpall.c
  • pgbench
  • psql
  • scripts
  • include
  • port

    • 15 files changed

    +138
    -142
    lines changed

    ‎contrib/oid2name/oid2name.c

    Lines changed: 6 additions & 7 deletions
    Original file line numberDiff line numberDiff line change
    @@ -261,7 +261,8 @@ PGconn *
    261261
    sql_conn(struct options * my_opts)
    262262
    {
    263263
    PGconn *conn;
    264-
    char *password = NULL;
    264+
    bool have_password = false;
    265+
    char password[100];
    265266
    bool new_pass;
    266267

    267268
    /*
    @@ -282,7 +283,7 @@ sql_conn(struct options * my_opts)
    282283
    keywords[2] = "user";
    283284
    values[2] = my_opts->username;
    284285
    keywords[3] = "password";
    285-
    values[3] = password;
    286+
    values[3] = have_password ? password : NULL;
    286287
    keywords[4] = "dbname";
    287288
    values[4] = my_opts->dbname;
    288289
    keywords[5] = "fallback_application_name";
    @@ -302,17 +303,15 @@ sql_conn(struct options * my_opts)
    302303

    303304
    if (PQstatus(conn) == CONNECTION_BAD &&
    304305
    PQconnectionNeedsPassword(conn) &&
    305-
    password == NULL)
    306+
    !have_password)
    306307
    {
    307308
    PQfinish(conn);
    308-
    password = simple_prompt("Password: ", 100, false);
    309+
    simple_prompt("Password: ", password, sizeof(password), false);
    310+
    have_password = true;
    309311
    new_pass = true;
    310312
    }
    311313
    } while (new_pass);
    312314

    313-
    if (password)
    314-
    free(password);
    315-
    316315
    /* check to see that the backend connection was successfully made */
    317316
    if (PQstatus(conn) == CONNECTION_BAD)
    318317
    {

    ‎contrib/vacuumlo/vacuumlo.c

    Lines changed: 11 additions & 6 deletions
    Original file line numberDiff line numberDiff line change
    @@ -65,13 +65,17 @@ vacuumlo(const char *database, const struct _param * param)
    6565
    long matched;
    6666
    long deleted;
    6767
    int i;
    68-
    static char *password = NULL;
    6968
    bool new_pass;
    7069
    bool success = true;
    70+
    static bool have_password = false;
    71+
    static char password[100];
    7172

    7273
    /* Note: password can be carried over from a previous call */
    73-
    if (param->pg_prompt == TRI_YES && password == NULL)
    74-
    password = simple_prompt("Password: ", 100, false);
    74+
    if (param->pg_prompt == TRI_YES && !have_password)
    75+
    {
    76+
    simple_prompt("Password: ", password, sizeof(password), false);
    77+
    have_password = true;
    78+
    }
    7579

    7680
    /*
    7781
    * Start the connection. Loop until we have a password if requested by
    @@ -91,7 +95,7 @@ vacuumlo(const char *database, const struct _param * param)
    9195
    keywords[2] = "user";
    9296
    values[2] = param->pg_user;
    9397
    keywords[3] = "password";
    94-
    values[3] = password;
    98+
    values[3] = have_password ? password : NULL;
    9599
    keywords[4] = "dbname";
    96100
    values[4] = database;
    97101
    keywords[5] = "fallback_application_name";
    @@ -110,11 +114,12 @@ vacuumlo(const char *database, const struct _param * param)
    110114

    111115
    if (PQstatus(conn) == CONNECTION_BAD &&
    112116
    PQconnectionNeedsPassword(conn) &&
    113-
    password == NULL &&
    117+
    !have_password &&
    114118
    param->pg_prompt != TRI_NO)
    115119
    {
    116120
    PQfinish(conn);
    117-
    password = simple_prompt("Password: ", 100, false);
    121+
    simple_prompt("Password: ", password, sizeof(password), false);
    122+
    have_password = true;
    118123
    new_pass = true;
    119124
    }
    120125
    } while (new_pass);

    ‎src/bin/initdb/initdb.c

    Lines changed: 9 additions & 14 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1557,8 +1557,8 @@ setup_auth(FILE *cmdfd)
    15571557
    static void
    15581558
    get_su_pwd(void)
    15591559
    {
    1560-
    char *pwd1,
    1561-
    *pwd2;
    1560+
    char pwd1[100];
    1561+
    char pwd2[100];
    15621562

    15631563
    if (pwprompt)
    15641564
    {
    @@ -1567,14 +1567,13 @@ get_su_pwd(void)
    15671567
    */
    15681568
    printf("\n");
    15691569
    fflush(stdout);
    1570-
    pwd1 = simple_prompt("Enter new superuser password: ", 100, false);
    1571-
    pwd2 = simple_prompt("Enter it again: ", 100, false);
    1570+
    simple_prompt("Enter new superuser password: ", pwd1, sizeof(pwd1), false);
    1571+
    simple_prompt("Enter it again: ", pwd2, sizeof(pwd2), false);
    15721572
    if (strcmp(pwd1, pwd2) != 0)
    15731573
    {
    15741574
    fprintf(stderr, _("Passwords didn't match.\n"));
    15751575
    exit_nicely();
    15761576
    }
    1577-
    free(pwd2);
    15781577
    }
    15791578
    else
    15801579
    {
    @@ -1587,7 +1586,6 @@ get_su_pwd(void)
    15871586
    * for now.
    15881587
    */
    15891588
    FILE *pwf = fopen(pwfilename, "r");
    1590-
    char pwdbuf[MAXPGPATH];
    15911589
    int i;
    15921590

    15931591
    if (!pwf)
    @@ -1596,7 +1594,7 @@ get_su_pwd(void)
    15961594
    progname, pwfilename, strerror(errno));
    15971595
    exit_nicely();
    15981596
    }
    1599-
    if (!fgets(pwdbuf, sizeof(pwdbuf), pwf))
    1597+
    if (!fgets(pwd1, sizeof(pwd1), pwf))
    16001598
    {
    16011599
    if (ferror(pwf))
    16021600
    fprintf(stderr, _("%s: could not read password from file \"%s\": %s\n"),
    @@ -1608,15 +1606,12 @@ get_su_pwd(void)
    16081606
    }
    16091607
    fclose(pwf);
    16101608

    1611-
    i = strlen(pwdbuf);
    1612-
    while (i > 0 && (pwdbuf[i - 1] == '\r' || pwdbuf[i - 1] == '\n'))
    1613-
    pwdbuf[--i] = '\0';
    1614-
    1615-
    pwd1 = pg_strdup(pwdbuf);
    1616-
    1609+
    i = strlen(pwd1);
    1610+
    while (i > 0 && (pwd1[i - 1] == '\r' || pwd1[i - 1] == '\n'))
    1611+
    pwd1[--i] = '\0';
    16171612
    }
    16181613

    1619-
    superuser_password = pwd1;
    1614+
    superuser_password = pg_strdup(pwd1);
    16201615
    }
    16211616

    16221617
    /*

    ‎src/bin/pg_basebackup/nls.mk

    Lines changed: 1 addition & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -2,3 +2,4 @@
    22
    CATALOG_NAME = pg_basebackup
    33
    AVAIL_LANGUAGES = de es fr it ko pl pt_BR ru zh_CN
    44
    GETTEXT_FILES = pg_basebackup.c pg_receivexlog.c pg_recvlogical.c receivelog.c streamutil.c ../../common/fe_memutils.c
    5+
    GETTEXT_TRIGGERS = simple_prompt

    ‎src/bin/pg_basebackup/streamutil.c

    Lines changed: 7 additions & 7 deletions
    Original file line numberDiff line numberDiff line change
    @@ -41,7 +41,8 @@ char *dbport = NULL;
    4141
    char *replication_slot = NULL;
    4242
    char *dbname = NULL;
    4343
    int dbgetpassword = 0; /* 0=auto, -1=never, 1=always */
    44-
    static char *dbpassword = NULL;
    44+
    static bool have_password = false;
    45+
    static char password[100];
    4546
    PGconn *conn = NULL;
    4647

    4748
    /*
    @@ -141,24 +142,23 @@ GetConnection(void)
    141142
    }
    142143

    143144
    /* If -W was given, force prompt for password, but only the first time */
    144-
    need_password = (dbgetpassword == 1 && dbpassword == NULL);
    145+
    need_password = (dbgetpassword == 1 && !have_password);
    145146

    146147
    do
    147148
    {
    148149
    /* Get a new password if appropriate */
    149150
    if (need_password)
    150151
    {
    151-
    if (dbpassword)
    152-
    free(dbpassword);
    153-
    dbpassword = simple_prompt(_("Password: "), 100, false);
    152+
    simple_prompt("Password: ", password, sizeof(password), false);
    153+
    have_password = true;
    154154
    need_password = false;
    155155
    }
    156156

    157157
    /* Use (or reuse, on a subsequent connection) password if we have it */
    158-
    if (dbpassword)
    158+
    if (have_password)
    159159
    {
    160160
    keywords[i] = "password";
    161-
    values[i] = dbpassword;
    161+
    values[i] = password;
    162162
    }
    163163
    else
    164164
    {

    ‎src/bin/pg_dump/pg_backup_db.c

    Lines changed: 14 additions & 21 deletions
    Original file line numberDiff line numberDiff line change
    @@ -134,6 +134,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
    134134
    const char *newdb;
    135135
    const char *newuser;
    136136
    char *password;
    137+
    char passbuf[100];
    137138
    bool new_pass;
    138139

    139140
    if (!reqdb)
    @@ -149,13 +150,12 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
    149150
    ahlog(AH, 1, "connecting to database \"%s\" as user \"%s\"\n",
    150151
    newdb, newuser);
    151152

    152-
    password = AH->savedPassword ? pg_strdup(AH->savedPassword) : NULL;
    153+
    password = AH->savedPassword;
    153154

    154155
    if (AH->promptPassword == TRI_YES && password == NULL)
    155156
    {
    156-
    password = simple_prompt("Password: ", 100, false);
    157-
    if (password == NULL)
    158-
    exit_horribly(modulename, "out of memory\n");
    157+
    simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
    158+
    password = passbuf;
    159159
    }
    160160

    161161
    initPQExpBuffer(&connstr);
    @@ -201,16 +201,14 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
    201201
    fprintf(stderr, "Connecting to %s as %s\n",
    202202
    newdb, newuser);
    203203

    204-
    if (password)
    205-
    free(password);
    206-
    207204
    if (AH->promptPassword != TRI_NO)
    208-
    password = simple_prompt("Password: ", 100, false);
    205+
    {
    206+
    simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
    207+
    password = passbuf;
    208+
    }
    209209
    else
    210210
    exit_horribly(modulename, "connection needs password\n");
    211211

    212-
    if (password == NULL)
    213-
    exit_horribly(modulename, "out of memory\n");
    214212
    new_pass = true;
    215213
    }
    216214
    } while (new_pass);
    @@ -225,8 +223,6 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
    225223
    free(AH->savedPassword);
    226224
    AH->savedPassword = pg_strdup(PQpass(newConn));
    227225
    }
    228-
    if (password)
    229-
    free(password);
    230226

    231227
    termPQExpBuffer(&connstr);
    232228

    @@ -258,18 +254,18 @@ ConnectDatabase(Archive *AHX,
    258254
    {
    259255
    ArchiveHandle *AH = (ArchiveHandle *) AHX;
    260256
    char *password;
    257+
    char passbuf[100];
    261258
    bool new_pass;
    262259

    263260
    if (AH->connection)
    264261
    exit_horribly(modulename, "already connected to a database\n");
    265262

    266-
    password = AH->savedPassword ? pg_strdup(AH->savedPassword) : NULL;
    263+
    password = AH->savedPassword;
    267264

    268265
    if (prompt_password == TRI_YES && password == NULL)
    269266
    {
    270-
    password = simple_prompt("Password: ", 100, false);
    271-
    if (password == NULL)
    272-
    exit_horribly(modulename, "out of memory\n");
    267+
    simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
    268+
    password = passbuf;
    273269
    }
    274270
    AH->promptPassword = prompt_password;
    275271

    @@ -309,9 +305,8 @@ ConnectDatabase(Archive *AHX,
    309305
    prompt_password != TRI_NO)
    310306
    {
    311307
    PQfinish(AH->connection);
    312-
    password = simple_prompt("Password: ", 100, false);
    313-
    if (password == NULL)
    314-
    exit_horribly(modulename, "out of memory\n");
    308+
    simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
    309+
    password = passbuf;
    315310
    new_pass = true;
    316311
    }
    317312
    } while (new_pass);
    @@ -332,8 +327,6 @@ ConnectDatabase(Archive *AHX,
    332327
    free(AH->savedPassword);
    333328
    AH->savedPassword = pg_strdup(PQpass(AH->connection));
    334329
    }
    335-
    if (password)
    336-
    free(password);
    337330

    338331
    /* check for version mismatch */
    339332
    _check_database_version(AH);

    ‎src/bin/pg_dump/pg_dumpall.c

    Lines changed: 11 additions & 6 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1884,13 +1884,17 @@ connectDatabase(const char *dbname, const char *connection_string,
    18841884
    bool new_pass;
    18851885
    const char *remoteversion_str;
    18861886
    int my_version;
    1887-
    static char *password = NULL;
    18881887
    const char **keywords = NULL;
    18891888
    const char **values = NULL;
    18901889
    PQconninfoOption *conn_opts = NULL;
    1890+
    static bool have_password = false;
    1891+
    static char password[100];
    18911892

    1892-
    if (prompt_password == TRI_YES && !password)
    1893-
    password = simple_prompt("Password: ", 100, false);
    1893+
    if (prompt_password == TRI_YES && !have_password)
    1894+
    {
    1895+
    simple_prompt("Password: ", password, sizeof(password), false);
    1896+
    have_password = true;
    1897+
    }
    18941898

    18951899
    /*
    18961900
    * Start the connection. Loop until we have a password if requested by
    @@ -1970,7 +1974,7 @@ connectDatabase(const char *dbname, const char *connection_string,
    19701974
    values[i] = pguser;
    19711975
    i++;
    19721976
    }
    1973-
    if (password)
    1977+
    if (have_password)
    19741978
    {
    19751979
    keywords[i] = "password";
    19761980
    values[i] = password;
    @@ -1998,11 +2002,12 @@ connectDatabase(const char *dbname, const char *connection_string,
    19982002

    19992003
    if (PQstatus(conn) == CONNECTION_BAD &&
    20002004
    PQconnectionNeedsPassword(conn) &&
    2001-
    password == NULL &&
    2005+
    !have_password &&
    20022006
    prompt_password != TRI_NO)
    20032007
    {
    20042008
    PQfinish(conn);
    2005-
    password = simple_prompt("Password: ", 100, false);
    2009+
    simple_prompt("Password: ", password, sizeof(password), false);
    2010+
    have_password = true;
    20062011
    new_pass = true;
    20072012
    }
    20082013
    } while (new_pass);

    ‎src/bin/pgbench/pgbench.c

    Lines changed: 6 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -773,8 +773,9 @@ static PGconn *
    773773
    doConnect(void)
    774774
    {
    775775
    PGconn *conn;
    776-
    static char *password = NULL;
    777776
    bool new_pass;
    777+
    static bool have_password = false;
    778+
    static char password[100];
    778779

    779780
    /*
    780781
    * Start the connection. Loop until we have a password if requested by
    @@ -794,7 +795,7 @@ doConnect(void)
    794795
    keywords[2] = "user";
    795796
    values[2] = login;
    796797
    keywords[3] = "password";
    797-
    values[3] = password;
    798+
    values[3] = have_password ? password : NULL;
    798799
    keywords[4] = "dbname";
    799800
    values[4] = dbName;
    800801
    keywords[5] = "fallback_application_name";
    @@ -815,10 +816,11 @@ doConnect(void)
    815816

    816817
    if (PQstatus(conn) == CONNECTION_BAD &&
    817818
    PQconnectionNeedsPassword(conn) &&
    818-
    password == NULL)
    819+
    !have_password)
    819820
    {
    820821
    PQfinish(conn);
    821-
    password = simple_prompt("Password: ", 100, false);
    822+
    simple_prompt("Password: ", password, sizeof(password), false);
    823+
    have_password = true;
    822824
    new_pass = true;
    823825
    }
    824826
    } while (new_pass);

    0 commit comments

    Comments
     (0)
    0