Re: Incorrect usage of strtol, atoi for non-numeric junk inputs - Mailing list pgsql-hackers
| From | Kyotaro Horiguchi |
|---|---|
| Subject | Re: Incorrect usage of strtol, atoi for non-numeric junk inputs |
| Date | |
| Msg-id | 20210714.103556.2157482390190763832.horikyota.ntt@gmail.com Whole thread Raw |
| In response to | Incorrect usage of strtol, atoi for non-numeric junk inputs (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>) |
| List | pgsql-hackers |
Thanks for the discussion.
At Tue, 13 Jul 2021 09:28:30 +0900, Michael Paquier <michael@paquier.xyz> wrote in
> On Fri, Jul 09, 2021 at 04:50:28PM +0900, Kyotaro Horiguchi wrote:
> > At Fri, 9 Jul 2021 10:29:07 +0900, Michael Paquier <michael@paquier.xyz> wrote in
> >> Er, wait. We've actually allowed negative values for pg_ctl
> >> --timeout or the subcommand kill!?
> >
> > --timeout accepts values less than 1, which values cause the command
> > ends with the timeout error before checking for the ending state. Not
> > destructive but useless as a behavior. We have two choices here. One
> > is simply inhibiting zero or negative timeouts, and another is
> > allowing zero as timeout and giving it the same meaning to
> > --no-wait. The former is strictly right behavior but the latter is
> > casual and convenient. I took the former way in this version.
>
> Yeah, that's not useful.
^^; Ok, I'm fine with taking the second way. Changed it.
"-t 0" means "-W" in the attached and that behavior is described in
the doc part. The environment variable PGCTLTIMEOUT accepts the same
range of values. I added a warning that notifies that -t 0 overrides
explicit -w .
> $ pg_ctl -w -t 0 start
> pg_ctl: WARNING: -w is ignored because timeout is set to 0
> server starting
> >> This one in pg_upgrade is incomplete. Perhaps the missing comment
> >> should tell that negative job values are checked later on?
> >
> > Zero or negative job numbers mean non-parallel mode and don't lead to
> > an error. If we don't need to preserve that behavior (I personally
> > don't think it is ether useful and/or breaks someone's existing
> > usage.), it is sensible to do range-check here.
>
> Hmm. It would be good to preserve some compatibility here, but I can
> see more benefits in being consistent between all the tools, and make
> people aware that such commands are not generated more carefully.
Ageed.
> > case 'j':
> > - opts.jobs = atoi(optarg);
> > - if (opts.jobs < 1)
> > + errno = 0;
> > + opts.jobs = strtoint(optarg, &endptr, 10);
> > + if (*endptr || errno == ERANGE || opts.jobs < 1)
> > {
> > pg_log_error("number of parallel jobs must be at least 1");
> > exit(1);
>
> specifying a value that triggers ERANGE could be confusing for values
> higher than INT_MAX with pg_amcheck --jobs:
> $ pg_amcheck --jobs=4000000000
> pg_amcheck: error: number of parallel jobs must be at least 1
> I think that this should be reworded as "invalid number of parallel
> jobs \"$s\"", or "number of parallel jobs must be in range %d..%d".
> Perhaps we could have a combination of both? Using the first message
> is useful with junk, non-numeric values or trailing characters. The
> second is useful to make users aware that the value is numeric, but
> not quite right.
Yeah, I noticed that but ignored as a kind of impossible, or
something-needless-to-say:p
> "invalid number of parallel jobs \"$s\""
> "number of parallel jobs must be in range %d..%d"
The resulting combined message looks like this:
> "number of parallel jobs must be an integer in range 1..2147483647"
I don't think it's not great that the message explicitly suggests a
limit like INT_MAX, which is far above the practical limit. So, (even
though I avoided to do that but) in the attached, I changed my mind to
split most of the errors into two messages to avoid suggesting such
impractical limits.
$ pg_amcheck -j -1
$ pg_amcheck -j 1x
$ pg_amcheck -j 10000000000000x
pg_amcheck: error: number of parallel jobs must be an integer greater than zero: "...."
$ pg_amcheck -j 10000000000000
pg_amcheck: error: number of parallel jobs out of range: "10000000000000"
If you feel it's too-much or pointless to split that way, I'll happy
to change it the "%d..%d" form.
Still I used the "%d..%d" notation for some parameters that has a
finite range by design. They look like the following:
(%d..%d doesn't work well for real numbers.)
"sampling rate must be an real number between 0.0 and 1.0: \"%s\""
I'm not sure what to do for numWorkers of pg_dump/restore. The limit
for numWorkers are lowered on Windows to quite low value, which would
be worth showing, but otherwise the limit is INT_MAX. The attached
makes pg_dump respond to -j 100 with the following error message which
doesn't suggest the finite limit 64 on Windows.
$ pg_dump -j 100
pg_dump: error: number of parallel jobs out of range: "100"
> > @@ -587,8 +602,10 @@ main(int argc, char **argv)
> >
> > case 8:
> > have_extra_float_digits = true;
> > - extra_float_digits = atoi(optarg);
> > - if (extra_float_digits < -15 || extra_float_digits > 3)
> > + errno = 0;
> > + extra_float_digits = strtoint(optarg, &endptr, 10);
> > + if (*endptr || errno == ERANGE ||
> > + extra_float_digits < -15 || extra_float_digits > 3)
> > {
> > pg_log_error("extra_float_digits must be in range -15..3");
> > exit_nicely(1);
>
> Should we take this occasion to reduce the burden of translators and
> reword that as "%s must be in range %d..%d"? That could be a separate
> patch.
The first %s is not always an invariable symbol name so it could
result in concatenating several phrases into one, like this.
pg_log..("%s must be in range %s..%s", _("compression level"), "0", "9"))
It is translatable at least into Japanese but I'm not sure about other
languages.
In the attached, most of the messages are not in this shape since I
avoided to suggest substantially infinite limits, thus this doesn't
fully work. I'll consider it if the current shape is found to be
unacceptable. In that case I'll use the option names in the messages
instead of the natural names.
> > case 'p':
> > - if ((old_cluster.port = atoi(optarg)) <= 0)
> > - pg_fatal("invalid old port number\n");
> > + errno = 0;
> > + if ((old_cluster.port = strtoint(optarg, &endptr, 10)) <= 0 ||
> > + *endptr || errno == ERANGE)
> > + pg_fatal("invalid old port number %s\n", optarg);
> > break;
>
> You may want to use columns here, or specify the port range:
> "invalid old port number: %s" or "old port number must be in range
> %d..%d".
I'm not sure whether the colons(?) are required, since pg_receivewal
currently complains as "invalid port number \"%s\"" but I agree that
it would be better if we had one.
By the way, in the attached version, the message is split into the
following combination. ("an integer" might be useless..)
pg_fatal("old port number must be an integer greater than zero: \"%s\"\n",
pg_fatal("old port number out of range: \"%s\"\n", optarg);
As the result.
> > + if (*endptr || errno == ERANGE || concurrentCons <= 0)
> > {
> > - pg_log_error("number of parallel jobs must be at least 1");
> > + pg_log_error("number of parallel jobs must be at least 1: %s", optarg);
> > exit(1);
> > }
>
> This one is also confusing with optorg > INT_MAX.
The attached version says just "out of range" in that case.
- Is it worth avoiding suggesting a virtually infinite upper limit by
splitting out "out of range" from an error message?
- If not, I'll use the single message "xxx must be in range
1..2147483647" or "xxx must be an integer in range 1..2147483647".
Do you think we need the parameter type "an integer" there?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 56d4fda06a9003efe1172848180114e42066db98 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Thu, 8 Jul 2021 15:08:01 +0900
Subject: [PATCH v3 1/3] Be strict in numeric parameters on command line
Some numeric command line parameters are tolerant of valid values
followed by garbage like "123xyz". Be strict to reject such invalid
values. Do the same for psql meta command parameters.
---
src/bin/pg_amcheck/pg_amcheck.c | 15 ++-
src/bin/pg_basebackup/pg_basebackup.c | 24 +++-
src/bin/pg_basebackup/pg_receivewal.c | 39 +++++--
src/bin/pg_basebackup/pg_recvlogical.c | 44 +++++--
src/bin/pg_checksums/pg_checksums.c | 17 ++-
src/bin/pg_ctl/pg_ctl.c | 42 ++++++-
src/bin/pg_dump/pg_dump.c | 57 ++++++---
src/bin/pg_dump/pg_restore.c | 42 ++++---
src/bin/pg_upgrade/option.c | 30 ++++-
src/bin/pgbench/pgbench.c | 154 +++++++++++++++++++------
src/bin/psql/command.c | 73 +++++++++++-
src/bin/scripts/reindexdb.c | 15 ++-
src/bin/scripts/vacuumdb.c | 59 ++++++++--
13 files changed, 484 insertions(+), 127 deletions(-)
diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 4bde16fb4b..f40d58ac96 100644
--- a/src/bin/pg_amcheck/pg_amcheck.c
+++ b/src/bin/pg_amcheck/pg_amcheck.c
@@ -17,6 +17,7 @@
#include "catalog/pg_am_d.h"
#include "catalog/pg_namespace_d.h"
#include "common/logging.h"
+#include "common/string.h"
#include "common/username.h"
#include "fe_utils/cancel.h"
#include "fe_utils/option_utils.h"
@@ -326,10 +327,18 @@ main(int argc, char *argv[])
append_btree_pattern(&opts.exclude, optarg, encoding);
break;
case 'j':
- opts.jobs = atoi(optarg);
- if (opts.jobs < 1)
+ errno = 0;
+ opts.jobs = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("number of parallel jobs must be at least 1");
+ pg_log_error("number of parallel jobs out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || opts.jobs < 1)
+ {
+ pg_log_error("number of parallel jobs must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
break;
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8bb0acf498..c30005f569 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2287,6 +2287,8 @@ main(int argc, char **argv)
while ((c = getopt_long(argc, argv, "CD:F:r:RS:T:X:l:nNzZ:d:c:h:p:U:s:wWkvP",
long_options, &option_index)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'C':
@@ -2371,10 +2373,12 @@ main(int argc, char **argv)
#endif
break;
case 'Z':
- compresslevel = atoi(optarg);
- if (compresslevel < 0 || compresslevel > 9)
+ errno = 0;
+ compresslevel = strtoint(optarg, &endptr, 10);
+ if (*endptr ||
+ errno == ERANGE || compresslevel < 0 || compresslevel > 9)
{
- pg_log_error("invalid compression level \"%s\"", optarg);
+ pg_log_error("compression level must be a digit in range 0..9: \"%s\"", optarg);
exit(1);
}
break;
@@ -2409,10 +2413,18 @@ main(int argc, char **argv)
dbgetpassword = 1;
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
+ errno = 0;
+ standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("invalid status interval \"%s\"", optarg);
+ pg_log_error("status interval out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || standby_message_timeout < 0)
+ {
+ pg_log_error("status interval must be a non-negative integer: \"%s\"",
+ optarg);
exit(1);
}
break;
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index c1334fad35..fb03147fe7 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -22,6 +22,7 @@
#include "access/xlog_internal.h"
#include "common/file_perm.h"
#include "common/logging.h"
+#include "common/string.h"
#include "getopt_long.h"
#include "libpq-fe.h"
#include "receivelog.h"
@@ -520,6 +521,9 @@ main(int argc, char **argv)
while ((c = getopt_long(argc, argv, "D:d:E:h:p:U:s:S:nwWvZ:",
long_options, &option_index)) != -1)
{
+ char *endptr;
+ int v;
+
switch (c)
{
case 'D':
@@ -532,9 +536,17 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
+ errno = 0;
+ v = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("invalid port number \"%s\"", optarg);
+ pg_log_error("port number out of range: \"%s\"", optarg);
+ exit(1);
+ }
+ if (*endptr || v < 1)
+ {
+ pg_log_error("port number must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
dbport = pg_strdup(optarg);
@@ -549,10 +561,18 @@ main(int argc, char **argv)
dbgetpassword = 1;
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
+ errno = 0;
+ standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("invalid status interval \"%s\"", optarg);
+ pg_log_error("status interval out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || standby_message_timeout < 0)
+ {
+ pg_log_error("status interval must be a non-negative integer: \"%s\"",
+ optarg);
exit(1);
}
break;
@@ -574,10 +594,13 @@ main(int argc, char **argv)
verbose++;
break;
case 'Z':
- compresslevel = atoi(optarg);
- if (compresslevel < 0 || compresslevel > 9)
+ errno = 0;
+ compresslevel = strtoint(optarg, &endptr, 10);
+ if (*endptr ||
+ errno == ERANGE || compresslevel < 0 || compresslevel > 9)
{
- pg_log_error("invalid compression level \"%s\"", optarg);
+ pg_log_error("compression level must be a digit in range 0..9: \"%s\"",
+ optarg);
exit(1);
}
break;
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 76bd153fac..9bc4902033 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -23,6 +23,7 @@
#include "common/fe_memutils.h"
#include "common/file_perm.h"
#include "common/logging.h"
+#include "common/string.h"
#include "getopt_long.h"
#include "libpq-fe.h"
#include "libpq/pqsignal.h"
@@ -732,6 +733,9 @@ main(int argc, char **argv)
while ((c = getopt_long(argc, argv, "E:f:F:nvtd:h:p:U:wWI:o:P:s:S:",
long_options, &option_index)) != -1)
{
+ char *endptr;
+ int v;
+
switch (c)
{
/* general options */
@@ -739,10 +743,18 @@ main(int argc, char **argv)
outfile = pg_strdup(optarg);
break;
case 'F':
- fsync_interval = atoi(optarg) * 1000;
- if (fsync_interval < 0)
+ errno = 0;
+ fsync_interval = strtoint(optarg, &endptr, 10) * 1000;
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("invalid fsync interval \"%s\"", optarg);
+ pg_log_error("fsync interval out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || fsync_interval < 0)
+ {
+ pg_log_error("fsync interval must be a non-negative integer: \"%s\"",
+ optarg);
exit(1);
}
break;
@@ -763,9 +775,17 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
+ errno = 0;
+ v = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("invalid port number \"%s\"", optarg);
+ pg_log_error("port number out of range: \"%s\"", optarg);
+ exit(1);
+ }
+ if (*endptr || v < 1)
+ {
+ pg_log_error("port number must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
dbport = pg_strdup(optarg);
@@ -820,10 +840,18 @@ main(int argc, char **argv)
plugin = pg_strdup(optarg);
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
+ errno = 0;
+ standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("invalid status interval \"%s\"", optarg);
+ pg_log_error("status interval out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || standby_message_timeout < 0)
+ {
+ pg_log_error("status interval must be a non-negative integer: \"%s\"",
+ optarg);
exit(1);
}
break;
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 3c326906e2..78a1d4ef38 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -24,6 +24,7 @@
#include "common/file_perm.h"
#include "common/file_utils.h"
#include "common/logging.h"
+#include "common/string.h"
#include "getopt_long.h"
#include "pg_getopt.h"
#include "storage/bufpage.h"
@@ -506,6 +507,9 @@ main(int argc, char *argv[])
while ((c = getopt_long(argc, argv, "cD:deNPf:v", long_options, &option_index)) != -1)
{
+ char *endptr;
+ int v;
+
switch (c)
{
case 'c':
@@ -518,9 +522,18 @@ main(int argc, char *argv[])
mode = PG_MODE_ENABLE;
break;
case 'f':
- if (atoi(optarg) == 0)
+ errno = 0;
+ v = strtoint(optarg, &endptr, 10);
+ if(*endptr == 0 && errno == ERANGE)
{
- pg_log_error("invalid filenode specification, must be numeric: %s", optarg);
+ pg_log_error("filenode specification out of range: %s",
+ optarg);
+ exit(1);
+ }
+ if(*endptr || v < 1)
+ {
+ pg_log_error("filenode specification must be an integer greater than zero: %s",
+ optarg);
exit(1);
}
only_filenode = pstrdup(optarg);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7985da0a94..0f72ef016b 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -76,6 +76,7 @@ typedef enum
#define WAITS_PER_SEC 10 /* should divide USEC_PER_SEC evenly */
+static bool do_wait_arg = false;
static bool do_wait = true;
static int wait_seconds = DEFAULT_WAIT;
static bool wait_seconds_arg = false;
@@ -2331,6 +2332,8 @@ main(int argc, char **argv)
/* process command-line options */
while (optind < argc)
{
+ char *endptr;
+
while ((c = getopt_long(argc, argv, "cD:e:l:m:N:o:p:P:sS:t:U:wW",
long_options, &option_index)) != -1)
{
@@ -2396,7 +2399,20 @@ main(int argc, char **argv)
#endif
break;
case 't':
- wait_seconds = atoi(optarg);
+ errno = 0;
+ wait_seconds = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
+ {
+ pg_log_error("timeout value out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || wait_seconds < 0)
+ {
+ pg_log_error("timeout value must be a non-negative integer: \"%s\"",
+ optarg);
+ exit(1);
+ }
wait_seconds_arg = true;
break;
case 'U':
@@ -2408,6 +2424,7 @@ main(int argc, char **argv)
break;
case 'w':
do_wait = true;
+ do_wait_arg = true;
break;
case 'W':
do_wait = false;
@@ -2459,7 +2476,20 @@ main(int argc, char **argv)
}
ctl_command = KILL_COMMAND;
set_sig(argv[++optind]);
- killproc = atol(argv[++optind]);
+ errno = 0;
+ killproc = strtol(argv[++optind], &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
+ {
+ pg_log_error("process ID out of range: \"%s\"",
+ argv[optind]);
+ exit(1);
+ }
+ if (*endptr || killproc < 0)
+ {
+ pg_log_error("process ID must be a non-negative integer: \"%s\"",
+ argv[optind]);
+ exit(1);
+ }
}
#ifdef WIN32
else if (strcmp(argv[optind], "register") == 0)
@@ -2514,6 +2544,14 @@ main(int argc, char **argv)
do_wait = false;
}
+ if (wait_seconds == 0 && do_wait)
+ {
+ /* Warn if user instructed to wait but we actually don't */
+ if (!silent_mode && do_wait_arg)
+ write_stderr(_("%s: WARNING: -w is ignored because timeout is set to 0\n"), progname);
+ do_wait = false;
+ }
+
if (pg_data)
{
snprintf(postopts_file, MAXPGPATH, "%s/postmaster.opts", pg_data);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 321152151d..8ef29b37f6 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -54,6 +54,7 @@
#include "catalog/pg_trigger_d.h"
#include "catalog/pg_type_d.h"
#include "common/connect.h"
+#include "common/string.h"
#include "dumputils.h"
#include "fe_utils/string_utils.h"
#include "getopt_long.h"
@@ -103,6 +104,17 @@ static Oid g_last_builtin_oid; /* value of the last builtin oid */
/* The specified names/patterns should to match at least one entity */
static int strict_names = 0;
+/*
+ * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS (= 64 usually)
+ * parallel jobs because that's the maximum limit for the
+ * WaitForMultipleObjects() call.
+ */
+#ifndef WIN32
+#define MAX_NUM_WORKERS INT_MAX
+#else
+#define MAX_NUM_WORKERS MAXIMUM_WAIT_OBJECTS
+#endif
+
/*
* Object inclusion/exclusion lists
*
@@ -486,7 +498,21 @@ main(int argc, char **argv)
break;
case 'j': /* number of dump jobs */
- numWorkers = atoi(optarg);
+ errno = 0;
+ numWorkers = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 &&
+ (errno == ERANGE || numWorkers > MAX_NUM_WORKERS))
+ {
+ pg_log_error("number of parallel jobs out of range: \"%s\"",
+ optarg);
+ exit_nicely(1);
+ }
+ if (*endptr || numWorkers <= 0)
+ {
+ pg_log_error("number of parallel jobs must be an integer greater than zero: \"%s\"",
+ optarg);
+ exit_nicely(1);
+ }
break;
case 'n': /* include schema(s) */
@@ -549,10 +575,12 @@ main(int argc, char **argv)
break;
case 'Z': /* Compression Level */
- compressLevel = atoi(optarg);
- if (compressLevel < 0 || compressLevel > 9)
+ errno = 0;
+ compressLevel = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE ||
+ compressLevel < 0 || compressLevel > 9)
{
- pg_log_error("compression level must be in range 0..9");
+ pg_log_error("compression level must be a digit in range 0..9: \"%s\"", optarg);
exit_nicely(1);
}
break;
@@ -587,10 +615,13 @@ main(int argc, char **argv)
case 8:
have_extra_float_digits = true;
- extra_float_digits = atoi(optarg);
- if (extra_float_digits < -15 || extra_float_digits > 3)
+ errno = 0;
+ extra_float_digits = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE ||
+ extra_float_digits < -15 || extra_float_digits > 3)
{
- pg_log_error("extra_float_digits must be in range -15..3");
+ pg_log_error("extra_float_digits must be an integer in range -15..3: \"%s\"",
+ optarg);
exit_nicely(1);
}
break;
@@ -719,18 +750,6 @@ main(int argc, char **argv)
if (!plainText)
dopt.outputCreateDB = 1;
- /*
- * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS (= 64 usually)
- * parallel jobs because that's the maximum limit for the
- * WaitForMultipleObjects() call.
- */
- if (numWorkers <= 0
-#ifdef WIN32
- || numWorkers > MAXIMUM_WAIT_OBJECTS
-#endif
- )
- fatal("invalid number of parallel jobs");
-
/* Parallel backup only in the directory archive format so far */
if (archiveFormat != archDirectory && numWorkers > 1)
fatal("parallel backup only supported by the directory format");
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 589b4aed53..3bb5a48c55 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -39,6 +39,7 @@
*-------------------------------------------------------------------------
*/
#include "postgres_fe.h"
+#include "common/string.h"
#include <ctype.h>
#ifdef HAVE_TERMIOS_H
@@ -52,6 +53,13 @@
static void usage(const char *progname);
+/* See comments in pg_dump.c */
+#ifndef WIN32
+#define MAX_NUM_WORKERS INT_MAX
+#else
+#define MAX_NUM_WORKERS MAXIMUM_WAIT_OBJECTS
+#endif
+
int
main(int argc, char **argv)
{
@@ -151,6 +159,8 @@ main(int argc, char **argv)
while ((c = getopt_long(argc, argv, "acCd:ef:F:h:I:j:lL:n:N:Op:P:RsS:t:T:U:vwWx1",
cmdopts, NULL)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'a': /* Dump data only */
@@ -181,7 +191,21 @@ main(int argc, char **argv)
break;
case 'j': /* number of restore jobs */
- numWorkers = atoi(optarg);
+ errno = 0;
+ numWorkers = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 &&
+ (errno == ERANGE || numWorkers > MAX_NUM_WORKERS))
+ {
+ pg_log_error("number of parallel jobs out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || numWorkers <= 0)
+ {
+ pg_log_error("number of parallel jobs must be an integer greater than zero: \"%s\"",
+ optarg);
+ exit(1);
+ }
break;
case 'l': /* Dump the TOC summary */
@@ -344,22 +368,6 @@ main(int argc, char **argv)
exit_nicely(1);
}
- if (numWorkers <= 0)
- {
- pg_log_error("invalid number of parallel jobs");
- exit(1);
- }
-
- /* See comments in pg_dump.c */
-#ifdef WIN32
- if (numWorkers > MAXIMUM_WAIT_OBJECTS)
- {
- pg_log_error("maximum number of parallel jobs is %d",
- MAXIMUM_WAIT_OBJECTS);
- exit(1);
- }
-#endif
-
/* Can't do single-txn mode with multiple connections */
if (opts->single_txn && numWorkers > 1)
{
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 64bbda5650..c014bbca0d 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -104,6 +104,8 @@ parseCommandLine(int argc, char *argv[])
while ((option = getopt_long(argc, argv, "d:D:b:B:cj:ko:O:p:P:rs:U:v",
long_options, &optindex)) != -1)
{
+ char *endptr;
+
switch (option)
{
case 'b':
@@ -127,7 +129,15 @@ parseCommandLine(int argc, char *argv[])
break;
case 'j':
- user_opts.jobs = atoi(optarg);
+ errno = 0;
+ user_opts.jobs = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
+ pg_fatal("number of parallel jobs out of range: \"%s\"\n",
+ optarg);
+ if (*endptr || user_opts.jobs < 1)
+ pg_fatal("number of parallel jobs must be an integer greater than zero: \"%s\"\n",
+ optarg);
+
break;
case 'k':
@@ -166,13 +176,23 @@ parseCommandLine(int argc, char *argv[])
* supported on all old/new versions (added in PG 9.2).
*/
case 'p':
- if ((old_cluster.port = atoi(optarg)) <= 0)
- pg_fatal("invalid old port number\n");
+ errno = 0;
+ old_cluster.port = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
+ pg_fatal("old port number out of range: \"%s\"\n", optarg);
+ if (*endptr || old_cluster.port <= 0)
+ pg_fatal("old port number must be an integer greater than zero: \"%s\"\n",
+ optarg);
break;
case 'P':
- if ((new_cluster.port = atoi(optarg)) <= 0)
- pg_fatal("invalid new port number\n");
+ errno = 0;
+ new_cluster.port = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
+ pg_fatal("new port number out of range: \"%s\"\n", optarg);
+ if (*endptr || new_cluster.port <= 0)
+ pg_fatal("new port number must be an integer greater than zero: \"%s\"\n",
+ optarg);
break;
case 'r':
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 364b5a2e47..1c3b5836c1 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5856,6 +5856,7 @@ main(int argc, char **argv)
while ((c = getopt_long(argc, argv, "iI:h:nvp:dqb:SNc:j:Crs:t:T:U:lf:D:F:M:P:R:L:", long_options, &optindex)) !=
-1)
{
char *script;
+ char *endptr;
switch (c)
{
@@ -5887,10 +5888,18 @@ main(int argc, char **argv)
break;
case 'c':
benchmarking_option_set = true;
- nclients = atoi(optarg);
- if (nclients <= 0)
+ errno = 0;
+ nclients = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_fatal("invalid number of clients: \"%s\"", optarg);
+ pg_log_fatal("number of clients out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || nclients <= 0)
+ {
+ pg_log_fatal("number of clients must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
#ifdef HAVE_GETRLIMIT
@@ -5914,10 +5923,18 @@ main(int argc, char **argv)
break;
case 'j': /* jobs */
benchmarking_option_set = true;
- nthreads = atoi(optarg);
- if (nthreads <= 0)
+ errno = 0;
+ nthreads = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_fatal("invalid number of threads: \"%s\"", optarg);
+ pg_log_fatal("number of threads out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || nthreads <= 0)
+ {
+ pg_log_fatal("number of threads must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
#ifndef ENABLE_THREAD_SAFETY
@@ -5938,28 +5955,50 @@ main(int argc, char **argv)
break;
case 's':
scale_given = true;
- scale = atoi(optarg);
- if (scale <= 0)
+ errno = 0;
+ scale = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_fatal("invalid scaling factor: \"%s\"", optarg);
+ pg_log_fatal("scaling factor out of range: \"%s\"", optarg);
+ exit(1);
+ }
+ if (*endptr || scale <= 0)
+ {
+ pg_log_fatal("scaling factor must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
break;
case 't':
benchmarking_option_set = true;
- nxacts = atoi(optarg);
- if (nxacts <= 0)
+ errno = 0;
+ nxacts = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_fatal("invalid number of transactions: \"%s\"", optarg);
+ pg_log_fatal("number of transactions out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || nxacts <= 0)
+ {
+ pg_log_fatal("number of transactions must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
break;
case 'T':
benchmarking_option_set = true;
- duration = atoi(optarg);
- if (duration <= 0)
+ errno = 0;
+ duration = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_fatal("invalid duration: \"%s\"", optarg);
+ pg_log_fatal("duration out of range: \"%s\"", optarg);
+ exit(1);
+ }
+ if (*endptr || duration <= 0)
+ {
+ pg_log_fatal("duration must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
break;
@@ -6019,10 +6058,13 @@ main(int argc, char **argv)
break;
case 'F':
initialization_option_set = true;
- fillfactor = atoi(optarg);
- if (fillfactor < 10 || fillfactor > 100)
+ errno = 0;
+ fillfactor = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE ||
+ fillfactor < 10 || fillfactor > 100)
{
- pg_log_fatal("invalid fillfactor: \"%s\"", optarg);
+ pg_log_fatal("fillfactor must be an ineger between 10 and 100: \"%s\"",
+ optarg);
exit(1);
}
break;
@@ -6039,23 +6081,38 @@ main(int argc, char **argv)
break;
case 'P':
benchmarking_option_set = true;
- progress = atoi(optarg);
- if (progress <= 0)
+ errno = 0;
+ progress = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_fatal("invalid thread progress delay: \"%s\"", optarg);
+ pg_log_fatal("thread progress delay out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || progress <= 0)
+ {
+ pg_log_fatal("thread progress delay must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
break;
case 'R':
{
/* get a double from the beginning of option value */
- double throttle_value = atof(optarg);
+ double throttle_value;
+ errno = 0;
+ throttle_value = strtod(optarg, &endptr);
benchmarking_option_set = true;
- if (throttle_value <= 0.0)
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_fatal("invalid rate limit: \"%s\"", optarg);
+ pg_log_fatal("rate limit out of range: \"%s\"", optarg);
+ exit(1);
+ }
+ if (*endptr || throttle_value <= 0.0)
+ {
+ pg_log_fatal("rate limit must be a real number greater than zero: \"%s\"", optarg);
exit(1);
}
/* Invert rate limit into per-transaction delay in usec */
@@ -6064,11 +6121,20 @@ main(int argc, char **argv)
break;
case 'L':
{
- double limit_ms = atof(optarg);
+ double limit_ms;
- if (limit_ms <= 0.0)
+ errno = 0;
+ limit_ms = strtod(optarg, &endptr);
+
+ if (*endptr == 0 && errno == ERANGE)
+ {
+ pg_log_fatal("latency limit out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || limit_ms <= 0.0)
{
- pg_log_fatal("invalid latency limit: \"%s\"", optarg);
+ pg_log_fatal("latency limit must be a real number greater than zero: \"%s\"", optarg);
exit(1);
}
benchmarking_option_set = true;
@@ -6089,19 +6155,27 @@ main(int argc, char **argv)
break;
case 4: /* sampling-rate */
benchmarking_option_set = true;
- sample_rate = atof(optarg);
- if (sample_rate <= 0.0 || sample_rate > 1.0)
+ errno = 0;
+ sample_rate = strtod(optarg, &endptr);
+ if (*endptr || errno == ERANGE ||
+ sample_rate <= 0.0 || sample_rate > 1.0)
{
- pg_log_fatal("invalid sampling rate: \"%s\"", optarg);
+ pg_log_fatal("sampling rate must be an real number between 0.0 and 1.0: \"%s\"", optarg);
exit(1);
}
break;
case 5: /* aggregate-interval */
benchmarking_option_set = true;
- agg_interval = atoi(optarg);
- if (agg_interval <= 0)
+ errno = 0;
+ agg_interval = strtod(optarg, &endptr);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_fatal("invalid number of seconds for aggregation: \"%s\"", optarg);
+ pg_log_fatal("aggregate interval out of range: \"%s\"", optarg);
+ exit(1);
+ }
+ if (*endptr || agg_interval <= 0)
+ {
+ pg_log_fatal("aggregate interval must be a real number greater than zero: \"%s\"", optarg);
exit(1);
}
break;
@@ -6135,10 +6209,18 @@ main(int argc, char **argv)
break;
case 11: /* partitions */
initialization_option_set = true;
- partitions = atoi(optarg);
- if (partitions < 0)
+ errno = 0;
+ partitions = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_fatal("invalid number of partitions: \"%s\"", optarg);
+ pg_log_fatal("number of partitions out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || partitions < 0)
+ {
+ pg_log_fatal("number of partitions must be a non-negative integer: \"%s\"",
+ optarg);
exit(1);
}
break;
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index d704c4220c..13074051b3 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1040,10 +1040,18 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch,
}
if (ln)
{
- lineno = atoi(ln);
- if (lineno < 1)
+ char *endptr;
+
+ errno = 0;
+ lineno = strtoint(ln, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("invalid line number: %s", ln);
+ pg_log_error("line number out of range: %s", ln);
+ status = PSQL_CMD_ERROR;
+ }
+ if (*endptr || lineno < 1)
+ {
+ pg_log_error("line number must be an integer greater than zero: %s", ln);
status = PSQL_CMD_ERROR;
}
}
@@ -4284,7 +4292,25 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
else if (strcmp(param, "border") == 0)
{
if (value)
- popt->topt.border = atoi(value);
+ {
+ char *endptr;
+ int new_value;
+
+ errno = 0;
+ new_value = strtoint(value, &endptr, 10);
+ if (*endptr == 0 && (errno == ERANGE || new_value > 65535))
+ {
+ pg_log_error("\\pset: border out of range");
+ return false;
+ }
+ if (*endptr || new_value < 0)
+ {
+ pg_log_error("\\pset: border must be an integer greater than zero");
+ return false;
+ }
+
+ popt->topt.border = new_value;
+ }
}
/* set expanded/vertical mode */
@@ -4440,7 +4466,25 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
else if (strcmp(param, "pager_min_lines") == 0)
{
if (value)
- popt->topt.pager_min_lines = atoi(value);
+ {
+ char *endptr;
+ int new_value;
+
+ errno = 0;
+ new_value = strtoint(value, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
+ {
+ pg_log_error("\\pset: pager_min_lines out of range");
+ return false;
+ }
+ if (*endptr || new_value < 0)
+ {
+ pg_log_error("\\pset: pager_min_lines must be a non-negative integer");
+ return false;
+ }
+
+ popt->topt.pager_min_lines = new_value;
+ }
}
/* disable "(x rows)" footer */
@@ -4456,7 +4500,24 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
else if (strcmp(param, "columns") == 0)
{
if (value)
- popt->topt.columns = atoi(value);
+ {
+ char *endptr;
+ int new_value;
+
+ errno = 0;
+ new_value = strtoint(value, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
+ {
+ pg_log_error("\\pset: column out of range");
+ return false;
+ }
+ if (*endptr || new_value < 0)
+ {
+ pg_log_error("\\pset: column must be a non-negative integer");
+ return false;
+ }
+ popt->topt.columns = new_value;
+ }
}
else
{
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fc0681538a..42d4d20768 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -15,6 +15,7 @@
#include "common.h"
#include "common/connect.h"
#include "common/logging.h"
+#include "common/string.h"
#include "fe_utils/cancel.h"
#include "fe_utils/option_utils.h"
#include "fe_utils/parallel_slot.h"
@@ -109,6 +110,8 @@ main(int argc, char *argv[])
/* process command-line options */
while ((c = getopt_long(argc, argv, "h:p:U:wWeqS:d:ast:i:j:v", long_options, &optindex)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'h':
@@ -151,10 +154,16 @@ main(int argc, char *argv[])
simple_string_list_append(&indexes, optarg);
break;
case 'j':
- concurrentCons = atoi(optarg);
- if (concurrentCons <= 0)
+ errno = 0;
+ concurrentCons = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("number of parallel jobs must be at least 1");
+ pg_log_error("number of parallel jobs out of range: %s", optarg);
+ exit(1);
+ }
+ if (*endptr || concurrentCons <= 0)
+ {
+ pg_log_error("number of parallel jobs must be an integer greater than zero: %s", optarg);
exit(1);
}
break;
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 61974baa78..6b2a34edd0 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -17,6 +17,7 @@
#include "common.h"
#include "common/connect.h"
#include "common/logging.h"
+#include "common/string.h"
#include "fe_utils/cancel.h"
#include "fe_utils/option_utils.h"
#include "fe_utils/parallel_slot.h"
@@ -141,6 +142,8 @@ main(int argc, char *argv[])
while ((c = getopt_long(argc, argv, "h:p:U:wWeqd:zZFat:fvj:P:", long_options, &optindex)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'h':
@@ -192,18 +195,34 @@ main(int argc, char *argv[])
vacopts.verbose = true;
break;
case 'j':
- concurrentCons = atoi(optarg);
- if (concurrentCons <= 0)
+ errno = 0;
+ concurrentCons = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("number of parallel jobs must be at least 1");
+ pg_log_error("number of parallel jobs out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || concurrentCons <= 0)
+ {
+ pg_log_error("number of parallel jobs must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
break;
case 'P':
- vacopts.parallel_workers = atoi(optarg);
- if (vacopts.parallel_workers < 0)
+ errno = 0;
+ vacopts.parallel_workers = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("parallel workers for vacuum must be greater than or equal to zero");
+ pg_log_error("parallel workers out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || vacopts.parallel_workers < 0)
+ {
+ pg_log_error("parallel workers for vacuum must be a non-negative integer: \"%s\"",
+ optarg);
exit(1);
}
break;
@@ -220,18 +239,34 @@ main(int argc, char *argv[])
vacopts.skip_locked = true;
break;
case 6:
- vacopts.min_xid_age = atoi(optarg);
- if (vacopts.min_xid_age <= 0)
+ errno = 0;
+ vacopts.min_xid_age = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("minimum transaction ID age must be at least 1");
+ pg_log_error("minimum transaction ID age out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || vacopts.min_xid_age <= 0)
+ {
+ pg_log_error("minimum transaction ID age must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
break;
case 7:
- vacopts.min_mxid_age = atoi(optarg);
- if (vacopts.min_mxid_age <= 0)
+ errno = 0;
+ vacopts.min_mxid_age = strtoint(optarg, &endptr, 10);
+ if (*endptr == 0 && errno == ERANGE)
{
- pg_log_error("minimum multixact ID age must be at least 1");
+ pg_log_error("minimum multixact ID age out of range: \"%s\"",
+ optarg);
+ exit(1);
+ }
+ if (*endptr || vacopts.min_mxid_age <= 0)
+ {
+ pg_log_error("minimum multixact ID age must be an integer greater than zero: \"%s\"",
+ optarg);
exit(1);
}
break;
--
2.27.0
From 640b6b63ddc1c79b1013ba7f4956f8a0d62c40ae Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Thu, 8 Jul 2021 17:10:19 +0900
Subject: [PATCH v3 2/3] Make complain for invalid numeirc values in
environemnt variables
Postgresql commands that read environment variables for integer values
are tolerant of trailing garbages like '5432xyz'. Ignore such values
and make warning on it.
---
src/bin/pg_ctl/pg_ctl.c | 12 +++++++++++-
src/bin/pg_upgrade/option.c | 32 ++++++++++++++++++++++++++++++--
src/bin/psql/startup.c | 17 ++++++++++++++++-
3 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 0f72ef016b..0c5dba5ecf 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2319,7 +2319,17 @@ main(int argc, char **argv)
env_wait = getenv("PGCTLTIMEOUT");
if (env_wait != NULL)
- wait_seconds = atoi(env_wait);
+ {
+ char *endptr;
+ int tmp_wait;
+
+ errno = 0;
+ tmp_wait = strtoint(env_wait, &endptr, 10);
+ if (*endptr || errno == ERANGE || tmp_wait < 0)
+ write_stderr(_("%s: ignored invalid setting of environment variable PGCTLTIMEOUT: %s\n"), progname,
env_wait);
+ else
+ wait_seconds = tmp_wait;
+ }
/*
* 'Action' can be before or after args so loop over both. Some
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index c014bbca0d..0acb6b7cdc 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -71,8 +71,36 @@ parseCommandLine(int argc, char *argv[])
os_info.progname = get_progname(argv[0]);
/* Process libpq env. variables; load values here for usage() output */
- old_cluster.port = getenv("PGPORTOLD") ? atoi(getenv("PGPORTOLD")) : DEF_PGUPORT;
- new_cluster.port = getenv("PGPORTNEW") ? atoi(getenv("PGPORTNEW")) : DEF_PGUPORT;
+ old_cluster.port = new_cluster.port = DEF_PGUPORT;
+ if (getenv("PGPORTOLD"))
+ {
+ char *endptr;
+ int port;
+
+ errno = 0;
+ port = strtoint(getenv("PGPORTOLD"), &endptr, 10);
+ if (*endptr || errno == ERANGE || port < 1)
+ pg_log(PG_WARNING,
+ "%s: ignored invalid setting of environment variable PGPORTOLD: %s\n",
+ os_info.progname, getenv("PGPORTOLD"));
+ else
+ old_cluster.port = port;
+ }
+
+ if (getenv("PGPORTNEW"))
+ {
+ char *endptr;
+ int port;
+
+ errno = 0;
+ port = strtoint(getenv("PGPORTNEW"), &endptr, 10);
+ if (*endptr || errno == ERANGE || port < 1)
+ pg_log(PG_WARNING,
+ "%s: ignored invalid setting of environment variable PGPORTNEW: %s\n",
+ os_info.progname, getenv("PGPORTNEW"));
+ else
+ new_cluster.port = port;
+ }
os_user_effective_id = get_user_info(&os_info.user);
/* we override just the database user name; we got the OS id above */
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 5f36f0d1c6..35ccabce77 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -181,7 +181,22 @@ main(int argc, char *argv[])
refresh_utf8format(&(pset.popt.topt));
/* We must get COLUMNS here before readline() sets it */
- pset.popt.topt.env_columns = getenv("COLUMNS") ? atoi(getenv("COLUMNS")) : 0;
+ pset.popt.topt.env_columns = 0;
+ if (getenv("COLUMNS"))
+ {
+ char *endptr;
+ int cols;
+
+ errno = 0;
+ cols = strtoint(getenv("COLUMNS"), &endptr, 10);
+ if (*endptr || errno == ERANGE || cols < 0)
+ {
+ pg_log_warning("ignored invalid setting of environemt variable COLUMNS: %s",
+ getenv("COLUMNS"));
+ }
+
+ pset.popt.topt.env_columns = cols;
+ }
pset.notty = (!isatty(fileno(stdin)) || !isatty(fileno(stdout)));
--
2.27.0
From 4760039c811d2242bd517f72a515bac416d94710 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Tue, 13 Jul 2021 19:35:12 +0900
Subject: [PATCH v3 3/3] Doc change for pg_ctl
The previous patch change the bahavior of pg_ctl about
--timeout. Change the doc following the behavior change.
---
doc/src/sgml/ref/pg_ctl-ref.sgml | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
index 3946fa52ea..90e0fabc63 100644
--- a/doc/src/sgml/ref/pg_ctl-ref.sgml
+++ b/doc/src/sgml/ref/pg_ctl-ref.sgml
@@ -389,9 +389,11 @@ PostgreSQL documentation
<listitem>
<para>
Specifies the maximum number of seconds to wait when waiting for an
- operation to complete (see option <option>-w</option>). Defaults to
- the value of the <envar>PGCTLTIMEOUT</envar> environment variable or, if
- not set, to 60 seconds.
+ operation to complete (see option <option>-w</option>). If it is set
+ to zero, <command>pg_ctl</command> behaves the same way
+ with <option>-W</option>. Defaults to the value of
+ the <envar>PGCTLTIMEOUT</envar> environment variable or, if not set,
+ to 60 seconds.
</para>
</listitem>
</varlistentry>
--
2.27.0
pgsql-hackers by date: