public inbox for [email protected]
help / color / mirror / Atom feedRe: [HACKERS] Repetitive code in RI triggers
8+ messages / 5 participants
[nested] [flat]
* Re: [HACKERS] Repetitive code in RI triggers
@ 2017-11-18 21:31 Tom Lane <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2017-11-18 21:31 UTC (permalink / raw)
To: Ildar Musin <[email protected]>; +Cc: Maksim Milyutin <[email protected]>; Daniel Gustafsson <[email protected]>; pgsql-hackers; Peter Eisentraut <[email protected]>
Ildar Musin <[email protected]> writes:
> [ ri_triggers_v2.patch ]
Pushed with two minor improvements. I noticed that ri_setdefault could
just go directly to ri_restrict rather than call the two separate triggers
that would end up there anyway; that lets its argument be "TriggerData
*trigdata" for more consistency with the other cases. Also, this patch
made it very obvious that we were caching identical queries under hash
keys RI_PLAN_RESTRICT_DEL_CHECKREF and RI_PLAN_RESTRICT_UPD_CHECKREF,
so we might as well just use one hash entry for both cases, saving a few
lines of code as well as a lot of cycles. Likewise in the other two
functions.
regards, tom lane
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [HACKERS] Repetitive code in RI triggers
@ 2017-11-20 08:53 Ildar Musin <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Ildar Musin @ 2017-11-20 08:53 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Maksim Milyutin <[email protected]>; Daniel Gustafsson <[email protected]>; pgsql-hackers; Peter Eisentraut <[email protected]>
On 19.11.2017 00:31, Tom Lane wrote:
> Ildar Musin <[email protected]> writes:
>> [ ri_triggers_v2.patch ]
>
> Pushed with two minor improvements. I noticed that ri_setdefault could
> just go directly to ri_restrict rather than call the two separate triggers
> that would end up there anyway; that lets its argument be "TriggerData
> *trigdata" for more consistency with the other cases. Also, this patch
> made it very obvious that we were caching identical queries under hash
> keys RI_PLAN_RESTRICT_DEL_CHECKREF and RI_PLAN_RESTRICT_UPD_CHECKREF,
> so we might as well just use one hash entry for both cases, saving a few
> lines of code as well as a lot of cycles. Likewise in the other two
> functions.
>
> regards, tom lane
>
Great, thanks
--
Ildar Musin
[email protected]
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v2 1/2] Be strict in numeric parameters on command line
@ 2021-07-08 06:08 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Kyotaro Horiguchi @ 2021-07-08 06:08 UTC (permalink / raw)
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 | 6 ++-
src/bin/pg_basebackup/pg_basebackup.c | 13 +++--
src/bin/pg_basebackup/pg_receivewal.c | 18 +++++--
src/bin/pg_basebackup/pg_recvlogical.c | 17 +++++--
src/bin/pg_checksums/pg_checksums.c | 7 ++-
src/bin/pg_ctl/pg_ctl.c | 18 ++++++-
src/bin/pg_dump/pg_dump.c | 39 ++++++++-------
src/bin/pg_dump/pg_restore.c | 17 ++++---
src/bin/pg_upgrade/option.c | 21 ++++++--
src/bin/pgbench/pgbench.c | 66 ++++++++++++++++----------
src/bin/psql/command.c | 52 ++++++++++++++++++--
src/bin/scripts/reindexdb.c | 10 ++--
src/bin/scripts/vacuumdb.c | 23 +++++----
13 files changed, 219 insertions(+), 88 deletions(-)
diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 4bde16fb4b..71a82f9b75 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,8 +327,9 @@ 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 || errno == ERANGE || opts.jobs < 1)
{
pg_log_error("number of parallel jobs must be at least 1");
exit(1);
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8bb0acf498..29be95b96a 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,8 +2373,10 @@ 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);
exit(1);
@@ -2409,8 +2413,9 @@ 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 || errno == ERANGE || standby_message_timeout < 0)
{
pg_log_error("invalid status interval \"%s\"", optarg);
exit(1);
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index c1334fad35..7fef925b99 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,8 @@ 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;
+
switch (c)
{
case 'D':
@@ -532,7 +535,9 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
+ errno = 0;
+ if (strtoint(optarg, &endptr, 10) <= 0 ||
+ *endptr || errno == ERANGE)
{
pg_log_error("invalid port number \"%s\"", optarg);
exit(1);
@@ -549,8 +554,9 @@ 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 || errno == ERANGE || standby_message_timeout < 0)
{
pg_log_error("invalid status interval \"%s\"", optarg);
exit(1);
@@ -574,8 +580,10 @@ 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);
exit(1);
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 76bd153fac..7be932d025 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,8 @@ 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;
+
switch (c)
{
/* general options */
@@ -739,8 +742,9 @@ 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 || errno == ERANGE || fsync_interval < 0)
{
pg_log_error("invalid fsync interval \"%s\"", optarg);
exit(1);
@@ -763,7 +767,9 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
+ errno = 0;
+ if (strtoint(optarg, &endptr, 10) <= 0 ||
+ *endptr || errno == ERANGE)
{
pg_log_error("invalid port number \"%s\"", optarg);
exit(1);
@@ -820,8 +826,9 @@ 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 || errno == ERANGE || standby_message_timeout < 0)
{
pg_log_error("invalid status interval \"%s\"", optarg);
exit(1);
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 3c326906e2..1c4e5b9d85 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,8 @@ main(int argc, char *argv[])
while ((c = getopt_long(argc, argv, "cD:deNPf:v", long_options, &option_index)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'c':
@@ -518,7 +521,9 @@ main(int argc, char *argv[])
mode = PG_MODE_ENABLE;
break;
case 'f':
- if (atoi(optarg) == 0)
+ errno = 0;
+ if (strtoint(optarg, &endptr, 10) == 0
+ || *endptr || errno == ERANGE)
{
pg_log_error("invalid filenode specification, must be numeric: %s", optarg);
exit(1);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7985da0a94..d6a39182cf 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2331,6 +2331,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 +2398,13 @@ main(int argc, char **argv)
#endif
break;
case 't':
- wait_seconds = atoi(optarg);
+ errno = 0;
+ wait_seconds = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || wait_seconds < 1)
+ {
+ pg_log_error("invalid timeout value \"%s\", use --no-wait to finish without waiting", optarg);
+ exit(1);
+ }
wait_seconds_arg = true;
break;
case 'U':
@@ -2459,7 +2467,13 @@ 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 || errno == ERANGE || killproc < 0)
+ {
+ pg_log_error("invalid process ID \"%s\"", argv[optind]);
+ exit(1);
+ }
}
#ifdef WIN32
else if (strcmp(argv[optind], "register") == 0)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 321152151d..793f4b3509 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"
@@ -486,7 +487,19 @@ main(int argc, char **argv)
break;
case 'j': /* number of dump jobs */
- numWorkers = atoi(optarg);
+ errno = 0;
+ numWorkers = strtoint(optarg, &endptr, 10);
+ /*
+ * 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 (*endptr || errno == ERANGE || numWorkers <= 0
+#ifdef WIN32
+ || numWorkers > MAXIMUM_WAIT_OBJECTS
+#endif
+ )
+ fatal("invalid number of parallel jobs %s", optarg);
break;
case 'n': /* include schema(s) */
@@ -549,8 +562,10 @@ 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");
exit_nicely(1);
@@ -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);
@@ -719,18 +736,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..285a09aaac 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
@@ -151,6 +152,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 +184,13 @@ main(int argc, char **argv)
break;
case 'j': /* number of restore jobs */
- numWorkers = atoi(optarg);
+ errno = 0;
+ numWorkers = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || numWorkers <= 0)
+ {
+ pg_log_error("invalid number of parallel jobs");
+ exit(1);
+ }
break;
case 'l': /* Dump the TOC summary */
@@ -344,12 +353,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)
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 64bbda5650..f96f0d1e2a 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,12 @@ parseCommandLine(int argc, char *argv[])
break;
case 'j':
- user_opts.jobs = atoi(optarg);
+ errno = 0;
+ user_opts.jobs = strtoint(optarg, &endptr, 10);
+ /**/
+ if (*endptr || errno == ERANGE || user_opts.jobs < 1)
+ pg_fatal("invalid number of jobs %s\n", optarg);
+
break;
case 'k':
@@ -166,13 +173,17 @@ 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;
+ if ((old_cluster.port = strtoint(optarg, &endptr, 10)) <= 0 ||
+ *endptr || errno == ERANGE)
+ pg_fatal("invalid old port number %s\n", optarg);
break;
case 'P':
- if ((new_cluster.port = atoi(optarg)) <= 0)
- pg_fatal("invalid new port number\n");
+ errno = 0;
+ if ((new_cluster.port = strtoint(optarg, &endptr, 10)) <= 0 ||
+ *endptr || errno == ERANGE)
+ pg_fatal("invalid new port number %s\n", optarg);
break;
case 'r':
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 4aeccd93af..4020347585 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5838,6 +5838,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)
{
@@ -5869,8 +5870,9 @@ 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 || errno == ERANGE || nclients <= 0)
{
pg_log_fatal("invalid number of clients: \"%s\"", optarg);
exit(1);
@@ -5896,8 +5898,9 @@ 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 || errno == ERANGE || nthreads <= 0)
{
pg_log_fatal("invalid number of threads: \"%s\"", optarg);
exit(1);
@@ -5920,8 +5923,9 @@ 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 || errno == ERANGE || scale <= 0)
{
pg_log_fatal("invalid scaling factor: \"%s\"", optarg);
exit(1);
@@ -5929,8 +5933,9 @@ main(int argc, char **argv)
break;
case 't':
benchmarking_option_set = true;
- nxacts = atoi(optarg);
- if (nxacts <= 0)
+ errno = 0;
+ nxacts = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || nxacts <= 0)
{
pg_log_fatal("invalid number of transactions: \"%s\"", optarg);
exit(1);
@@ -5938,8 +5943,9 @@ main(int argc, char **argv)
break;
case 'T':
benchmarking_option_set = true;
- duration = atoi(optarg);
- if (duration <= 0)
+ errno = 0;
+ duration = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || duration <= 0)
{
pg_log_fatal("invalid duration: \"%s\"", optarg);
exit(1);
@@ -6001,8 +6007,10 @@ 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);
exit(1);
@@ -6021,8 +6029,9 @@ 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 || errno == ERANGE || progress <= 0)
{
pg_log_fatal("invalid thread progress delay: \"%s\"", optarg);
exit(1);
@@ -6031,11 +6040,13 @@ main(int argc, char **argv)
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 || errno == ERANGE || throttle_value <= 0.0)
{
pg_log_fatal("invalid rate limit: \"%s\"", optarg);
exit(1);
@@ -6046,9 +6057,12 @@ 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 || errno == ERANGE || limit_ms <= 0.0)
{
pg_log_fatal("invalid latency limit: \"%s\"", optarg);
exit(1);
@@ -6071,8 +6085,10 @@ 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);
exit(1);
@@ -6080,8 +6096,9 @@ main(int argc, char **argv)
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 || errno == ERANGE || agg_interval <= 0)
{
pg_log_fatal("invalid number of seconds for aggregation: \"%s\"", optarg);
exit(1);
@@ -6117,8 +6134,9 @@ 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 || errno == ERANGE || partitions < 0)
{
pg_log_fatal("invalid number of partitions: \"%s\"", optarg);
exit(1);
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 543401c6d6..aaed986ae1 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1039,8 +1039,11 @@ 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 || errno == ERANGE || lineno < 1)
{
pg_log_error("invalid line number: %s", ln);
status = PSQL_CMD_ERROR;
@@ -4283,7 +4286,21 @@ 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 || errno == ERANGE ||
+ new_value < 0 || new_value > 65535)
+ {
+ pg_log_error("\\pset: border is invalid or out of range");
+ return false;
+ }
+
+ popt->topt.border = new_value;
+ }
}
/* set expanded/vertical mode */
@@ -4439,7 +4456,20 @@ 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 || errno == ERANGE || new_value < 0)
+ {
+ pg_log_error("\\pset: pager_min_lines is invalid or out of range");
+ return false;
+ }
+
+ popt->topt.pager_min_lines = new_value;
+ }
}
/* disable "(x rows)" footer */
@@ -4455,7 +4485,19 @@ 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 || errno == ERANGE || new_value < 0)
+ {
+ pg_log_error("\\pset: column is invalid or out of range");
+ return false;
+ }
+ popt->topt.columns = new_value;
+ }
}
else
{
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fc0681538a..baa68d58d8 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,11 @@ 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 || 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);
}
break;
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 61974baa78..93b563998a 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,16 +195,18 @@ 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 || errno == ERANGE || concurrentCons <= 0)
{
pg_log_error("number of parallel jobs must be at least 1");
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 || errno == ERANGE || vacopts.parallel_workers < 0)
{
pg_log_error("parallel workers for vacuum must be greater than or equal to zero");
exit(1);
@@ -220,16 +225,18 @@ 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 || errno == ERANGE || vacopts.min_xid_age <= 0)
{
pg_log_error("minimum transaction ID age must be at least 1");
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 || errno == ERANGE || vacopts.min_mxid_age <= 0)
{
pg_log_error("minimum multixact ID age must be at least 1");
exit(1);
--
2.27.0
----Next_Part(Fri_Jul__9_16_50_28_2021_228)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v2-0002-Make-complain-for-invalid-numeirc-values-in-envir.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v3 1/3] Be strict in numeric parameters on command line
@ 2021-07-08 06:08 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Kyotaro Horiguchi @ 2021-07-08 06:08 UTC (permalink / raw)
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 | 9 +-
src/bin/pg_basebackup/pg_basebackup.c | 17 ++-
src/bin/pg_basebackup/pg_receivewal.c | 25 ++--
src/bin/pg_basebackup/pg_recvlogical.c | 30 ++---
src/bin/pg_checksums/Makefile | 4 +-
src/bin/pg_checksums/pg_checksums.c | 9 +-
src/bin/pg_ctl/Makefile | 4 +-
src/bin/pg_ctl/pg_ctl.c | 39 ++++++-
src/bin/pg_dump/pg_dump.c | 44 ++++---
src/bin/pg_dump/pg_restore.c | 29 ++---
src/bin/pg_upgrade/option.c | 24 +++-
src/bin/pgbench/pgbench.c | 119 ++++++++++---------
src/bin/psql/command.c | 63 ++++++++--
src/bin/scripts/reindexdb.c | 9 +-
src/bin/scripts/vacuumdb.c | 31 ++---
src/fe_utils/option_utils.c | 152 +++++++++++++++++++++++++
src/include/common/logging.h | 9 ++
src/include/fe_utils/option_utils.h | 23 ++++
18 files changed, 457 insertions(+), 183 deletions(-)
diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 4bde16fb4b..538436d4a5 100644
--- a/src/bin/pg_amcheck/pg_amcheck.c
+++ b/src/bin/pg_amcheck/pg_amcheck.c
@@ -12,11 +12,13 @@
*/
#include "postgres_fe.h"
+#include <limits.h>
#include <time.h>
#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,12 +328,9 @@ main(int argc, char *argv[])
append_btree_pattern(&opts.exclude, optarg, encoding);
break;
case 'j':
- opts.jobs = atoi(optarg);
- if (opts.jobs < 1)
- {
- pg_log_error("number of parallel jobs must be at least 1");
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-j/--jobs",
+ 1, INT_MAX, &opts.jobs))
exit(1);
- }
break;
case 'p':
port = pg_strdup(optarg);
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8bb0acf498..42a65ffbc3 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -15,6 +15,7 @@
#include <unistd.h>
#include <dirent.h>
+#include <limits.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <signal.h>
@@ -31,6 +32,7 @@
#include "common/file_utils.h"
#include "common/logging.h"
#include "common/string.h"
+#include "fe_utils/option_utils.h"
#include "fe_utils/recovery_gen.h"
#include "fe_utils/string_utils.h"
#include "getopt_long.h"
@@ -2371,12 +2373,9 @@ main(int argc, char **argv)
#endif
break;
case 'Z':
- compresslevel = atoi(optarg);
- if (compresslevel < 0 || compresslevel > 9)
- {
- pg_log_error("invalid compression level \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-Z/--compress",
+ 0, 9, &compresslevel))
exit(1);
- }
break;
case 'c':
if (pg_strcasecmp(optarg, "fast") == 0)
@@ -2409,12 +2408,10 @@ main(int argc, char **argv)
dbgetpassword = 1;
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
- {
- pg_log_error("invalid status interval \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg,
+ "-s/--status-interval",
+ 0, INT_MAX, &standby_message_timeout))
exit(1);
- }
break;
case 'v':
verbose++;
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index c1334fad35..1f976ad3eb 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -15,6 +15,7 @@
#include "postgres_fe.h"
#include <dirent.h>
+#include <limits.h>
#include <signal.h>
#include <sys/stat.h>
#include <unistd.h>
@@ -22,6 +23,8 @@
#include "access/xlog_internal.h"
#include "common/file_perm.h"
#include "common/logging.h"
+#include "common/string.h"
+#include "fe_utils/option_utils.h"
#include "getopt_long.h"
#include "libpq-fe.h"
#include "receivelog.h"
@@ -532,11 +535,10 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
- {
- pg_log_error("invalid port number \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-p/--port",
+ 1, 65535, NULL))
exit(1);
- }
+
dbport = pg_strdup(optarg);
break;
case 'U':
@@ -549,12 +551,10 @@ main(int argc, char **argv)
dbgetpassword = 1;
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
- {
- pg_log_error("invalid status interval \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg,
+ "-s/--status-interval",
+ 0, INT_MAX, &standby_message_timeout))
exit(1);
- }
break;
case 'S':
replication_slot = pg_strdup(optarg);
@@ -574,12 +574,9 @@ main(int argc, char **argv)
verbose++;
break;
case 'Z':
- compresslevel = atoi(optarg);
- if (compresslevel < 0 || compresslevel > 9)
- {
- pg_log_error("invalid compression level \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-Z/--compress",
+ 0, 9, &compresslevel))
exit(1);
- }
break;
/* action */
case 1:
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 76bd153fac..44622c002b 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -13,6 +13,7 @@
#include "postgres_fe.h"
#include <dirent.h>
+#include <limits.h>
#include <sys/stat.h>
#include <unistd.h>
#ifdef HAVE_SYS_SELECT_H
@@ -23,6 +24,8 @@
#include "common/fe_memutils.h"
#include "common/file_perm.h"
#include "common/logging.h"
+#include "common/string.h"
+#include "fe_utils/option_utils.h"
#include "getopt_long.h"
#include "libpq-fe.h"
#include "libpq/pqsignal.h"
@@ -739,12 +742,12 @@ main(int argc, char **argv)
outfile = pg_strdup(optarg);
break;
case 'F':
- fsync_interval = atoi(optarg) * 1000;
- if (fsync_interval < 0)
- {
- pg_log_error("invalid fsync interval \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg,
+ "-F/--fsync-interval",
+ 0, INT_MAX / 1000, &fsync_interval))
exit(1);
- }
+ /* convert to milliseconds */
+ fsync_interval *= 1000;
break;
case 'n':
noloop = 1;
@@ -763,11 +766,9 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
- {
- pg_log_error("invalid port number \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-p/--port",
+ 0, 65535, NULL))
exit(1);
- }
dbport = pg_strdup(optarg);
break;
case 'U':
@@ -820,12 +821,13 @@ main(int argc, char **argv)
plugin = pg_strdup(optarg);
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
- {
- pg_log_error("invalid status interval \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg,
+ "-s/--status-interval",
+ 0, INT_MAX / 1000,
+ &standby_message_timeout))
exit(1);
- }
+ /* convert to milliseconds */
+ standby_message_timeout *= 1000;
break;
case 'S':
replication_slot = pg_strdup(optarg);
diff --git a/src/bin/pg_checksums/Makefile b/src/bin/pg_checksums/Makefile
index ba62406105..a22cf107f9 100644
--- a/src/bin/pg_checksums/Makefile
+++ b/src/bin/pg_checksums/Makefile
@@ -15,13 +15,15 @@ subdir = src/bin/pg_checksums
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
+LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils
+
OBJS = \
$(WIN32RES) \
pg_checksums.o
all: pg_checksums
-pg_checksums: $(OBJS) | submake-libpgport
+pg_checksums: $(OBJS) | submake-libpgport submake-libpgfeutils
$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
install: all installdirs
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 3c326906e2..d62df9747a 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -15,6 +15,7 @@
#include "postgres_fe.h"
#include <dirent.h>
+#include <limits.h>
#include <time.h>
#include <sys/stat.h>
#include <unistd.h>
@@ -24,6 +25,8 @@
#include "common/file_perm.h"
#include "common/file_utils.h"
#include "common/logging.h"
+#include "common/string.h"
+#include "fe_utils/option_utils.h"
#include "getopt_long.h"
#include "pg_getopt.h"
#include "storage/bufpage.h"
@@ -518,11 +521,9 @@ main(int argc, char *argv[])
mode = PG_MODE_ENABLE;
break;
case 'f':
- if (atoi(optarg) == 0)
- {
- pg_log_error("invalid filenode specification, must be numeric: %s", optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-f/--filenode",
+ 1, INT_MAX, NULL))
exit(1);
- }
only_filenode = pstrdup(optarg);
break;
case 'N':
diff --git a/src/bin/pg_ctl/Makefile b/src/bin/pg_ctl/Makefile
index 5d5f5372a3..ed03d6dc7d 100644
--- a/src/bin/pg_ctl/Makefile
+++ b/src/bin/pg_ctl/Makefile
@@ -24,13 +24,15 @@ LDFLAGS_INTERNAL += $(libpq_pgport)
SUBMAKE_LIBPQ := submake-libpq
endif
+LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils
+
OBJS = \
$(WIN32RES) \
pg_ctl.o
all: pg_ctl
-pg_ctl: $(OBJS) | submake-libpgport $(SUBMAKE_LIBPQ)
+pg_ctl: $(OBJS) | submake-libpgport $(SUBMAKE_LIBPQ) submake-libpgfeutils
$(CC) $(CFLAGS) $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
install: all installdirs
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7985da0a94..f7ccdcdc96 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -12,6 +12,7 @@
#include "postgres_fe.h"
#include <fcntl.h>
+#include <limits.h>
#include <signal.h>
#include <time.h>
#include <sys/stat.h>
@@ -28,6 +29,7 @@
#include "common/file_perm.h"
#include "common/logging.h"
#include "common/string.h"
+#include "fe_utils/option_utils.h"
#include "getopt_long.h"
#include "utils/pidfile.h"
@@ -76,6 +78,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;
@@ -2396,7 +2399,9 @@ main(int argc, char **argv)
#endif
break;
case 't':
- wait_seconds = atoi(optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-t/--timeout",
+ 0, INT_MAX, &wait_seconds))
+ exit(1);
wait_seconds_arg = true;
break;
case 'U':
@@ -2408,6 +2413,7 @@ main(int argc, char **argv)
break;
case 'w':
do_wait = true;
+ do_wait_arg = true;
break;
case 'W':
do_wait = false;
@@ -2459,7 +2465,28 @@ main(int argc, char **argv)
}
ctl_command = KILL_COMMAND;
set_sig(argv[++optind]);
- killproc = atol(argv[++optind]);
+ switch (option_parse_long(PG_LOG_OMIT, argv[++optind], "PID",
+ 0, LONG_MAX, &killproc))
+ {
+ case OPTPARSE_SUCCESS:
+ break;
+ case OPTPARSE_MALFORMED:
+ write_stderr(_("%s: PID must be an integer: \"%s\"\n"),
+ progname, argv[optind]);
+ exit(1);
+ break;
+ case OPTPARSE_TOOSMALL:
+ write_stderr(_("%s: PID must be a non-negative integer: %s\n"),
+ progname, argv[optind]);
+ exit(1);
+ break;
+ case OPTPARSE_TOOLARGE:
+ case OPTPARSE_OUTOFRANGE:
+ write_stderr(_("%s: PID out of range: %s\n"),
+ progname, argv[optind]);
+ exit(1);
+ break;
+ }
}
#ifdef WIN32
else if (strcmp(argv[optind], "register") == 0)
@@ -2514,6 +2541,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 34b91bb226..41b360edc0 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -55,7 +55,9 @@
#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/option_utils.h"
#include "fe_utils/string_utils.h"
#include "getopt_long.h"
#include "libpq/libpq-fs.h"
@@ -104,6 +106,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
*
@@ -487,7 +500,9 @@ main(int argc, char **argv)
break;
case 'j': /* number of dump jobs */
- numWorkers = atoi(optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-j/--jobs",
+ 1, MAX_NUM_WORKERS, &numWorkers))
+ exit_nicely(1);
break;
case 'n': /* include schema(s) */
@@ -550,12 +565,9 @@ main(int argc, char **argv)
break;
case 'Z': /* Compression Level */
- compressLevel = atoi(optarg);
- if (compressLevel < 0 || compressLevel > 9)
- {
- pg_log_error("compression level must be in range 0..9");
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-Z/--compress",
+ 0, 9, &compressLevel))
exit_nicely(1);
- }
break;
case 0:
@@ -588,12 +600,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)
- {
- pg_log_error("extra_float_digits must be in range -15..3");
+ if (option_parse_int(PG_LOG_ERROR, optarg,
+ "--extra-float-digits",
+ -15, 3, &extra_float_digits))
exit_nicely(1);
- }
break;
case 9: /* inserts */
@@ -720,18 +730,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..6f4447027a 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -39,6 +39,8 @@
*-------------------------------------------------------------------------
*/
#include "postgres_fe.h"
+#include "common/string.h"
+#include "fe_utils/option_utils.h"
#include <ctype.h>
#ifdef HAVE_TERMIOS_H
@@ -52,6 +54,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)
{
@@ -181,7 +190,9 @@ main(int argc, char **argv)
break;
case 'j': /* number of restore jobs */
- numWorkers = atoi(optarg);
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-j/--jobs",
+ 1, MAX_NUM_WORKERS, &numWorkers))
+ exit(1);
break;
case 'l': /* Dump the TOC summary */
@@ -344,22 +355,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..a8cd23f7fa 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -9,12 +9,15 @@
#include "postgres_fe.h"
+#include <limits.h>
#include <time.h>
#ifdef WIN32
#include <io.h>
#endif
+#include "common/logging.h"
#include "common/string.h"
+#include "fe_utils/option_utils.h"
#include "getopt_long.h"
#include "pg_upgrade.h"
#include "utils/pidfile.h"
@@ -104,6 +107,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)
{
+ int p;
+
switch (option)
{
case 'b':
@@ -127,7 +132,10 @@ parseCommandLine(int argc, char *argv[])
break;
case 'j':
- user_opts.jobs = atoi(optarg);
+ if (option_parse_int(PG_LOG_OMIT, optarg, "-j/--jobs",
+ 0, INT_MAX, &user_opts.jobs))
+ pg_fatal("-j/--jobs must be an integer in range %d..%d: \"%s\"\n",
+ 0, INT_MAX, optarg);
break;
case 'k':
@@ -166,13 +174,19 @@ 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");
+ if (option_parse_int(PG_LOG_OMIT, optarg, "-p/--old-port",
+ 1, 65535, &p))
+ pg_fatal("old port number must be an integer in range %d..%d: \"%s\"\n",
+ 1, 65535, optarg);
+ old_cluster.port = p;
break;
case 'P':
- if ((new_cluster.port = atoi(optarg)) <= 0)
- pg_fatal("invalid new port number\n");
+ if (option_parse_int(PG_LOG_OMIT, optarg, "-P/--new-port",
+ 1, 65535, &p))
+ pg_fatal("new port number must be an integer in range %d..%d: \"%s\"\n",
+ 1, 65535, optarg);
+ new_cluster.port = p;
break;
case 'r':
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 364b5a2e47..faa0969d70 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -63,6 +63,7 @@
#include "common/username.h"
#include "fe_utils/cancel.h"
#include "fe_utils/conditional.h"
+#include "fe_utils/option_utils.h"
#include "fe_utils/string_utils.h"
#include "getopt_long.h"
#include "libpq-fe.h"
@@ -871,7 +872,12 @@ invalid_syntax:
return false;
}
-/* convert string to double, detecting overflows/underflows */
+/*
+ * convert string to double, detecting overflows/underflows
+ *
+ * This is similar to option_parse_double but leave this function for
+ * performance reasons.
+ */
bool
strtodouble(const char *str, bool errorOK, double *dv)
{
@@ -5887,12 +5893,9 @@ main(int argc, char **argv)
break;
case 'c':
benchmarking_option_set = true;
- nclients = atoi(optarg);
- if (nclients <= 0)
- {
- pg_log_fatal("invalid number of clients: \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_FATAL, optarg, "-c/--client",
+ 1, INT_MAX, &nclients))
exit(1);
- }
#ifdef HAVE_GETRLIMIT
#ifdef RLIMIT_NOFILE /* most platforms use RLIMIT_NOFILE */
if (getrlimit(RLIMIT_NOFILE, &rlim) == -1)
@@ -5914,12 +5917,10 @@ main(int argc, char **argv)
break;
case 'j': /* jobs */
benchmarking_option_set = true;
- nthreads = atoi(optarg);
- if (nthreads <= 0)
- {
- pg_log_fatal("invalid number of threads: \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_FATAL, optarg, "-j/--jobs",
+ 0, INT_MAX, &nthreads))
exit(1);
- }
+
#ifndef ENABLE_THREAD_SAFETY
if (nthreads != 1)
{
@@ -5938,30 +5939,21 @@ main(int argc, char **argv)
break;
case 's':
scale_given = true;
- scale = atoi(optarg);
- if (scale <= 0)
- {
- pg_log_fatal("invalid scaling factor: \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_FATAL, optarg, "-s/--scale",
+ 1, INT_MAX, &scale))
exit(1);
- }
break;
case 't':
benchmarking_option_set = true;
- nxacts = atoi(optarg);
- if (nxacts <= 0)
- {
- pg_log_fatal("invalid number of transactions: \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_FATAL, optarg, "-t/--transactions",
+ 1, INT_MAX, &nxacts))
exit(1);
- }
break;
case 'T':
benchmarking_option_set = true;
- duration = atoi(optarg);
- if (duration <= 0)
- {
- pg_log_fatal("invalid duration: \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_FATAL, optarg, "-T/--time",
+ 1, INT_MAX, &duration))
exit(1);
- }
break;
case 'U':
username = pg_strdup(optarg);
@@ -6019,12 +6011,9 @@ main(int argc, char **argv)
break;
case 'F':
initialization_option_set = true;
- fillfactor = atoi(optarg);
- if (fillfactor < 10 || fillfactor > 100)
- {
- pg_log_fatal("invalid fillfactor: \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_FATAL, optarg, "-F/--fillfactor",
+ 10, 100, &fillfactor))
exit(1);
- }
break;
case 'M':
benchmarking_option_set = true;
@@ -6039,38 +6028,61 @@ main(int argc, char **argv)
break;
case 'P':
benchmarking_option_set = true;
- progress = atoi(optarg);
- if (progress <= 0)
- {
- pg_log_fatal("invalid thread progress delay: \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_FATAL, optarg, "-P/--progress",
+ 1, INT_MAX, &progress))
exit(1);
- }
break;
case 'R':
{
- /* get a double from the beginning of option value */
- double throttle_value = atof(optarg);
+ double throttle_value;
- benchmarking_option_set = true;
+ if (option_parse_double(PG_LOG_FATAL, optarg, "-R/--rate",
+ &throttle_value))
+ exit(1);
- if (throttle_value <= 0.0)
+ /*
+ * This value will be used for yielding int64 number
+ * through some arithmetic. Quite arbitrary but people
+ * never want such small numbers for this parameter.
+ */
+ if (throttle_value <= 1e-5)
{
- pg_log_fatal("invalid rate limit: \"%s\"", optarg);
+ pg_log_fatal("-R/--rate must be greater than 1e-5: \"%s\"", optarg);
exit(1);
}
+
/* Invert rate limit into per-transaction delay in usec */
throttle_delay = 1000000.0 / throttle_value;
}
break;
case 'L':
{
- double limit_ms = atof(optarg);
+ double limit_ms;
+ if (option_parse_double(PG_LOG_FATAL, optarg,
+ "-L/--latency-limit", &limit_ms))
+ exit(1);
+
+ /* limit limit_ms so that latency_limit fits in int64 */
if (limit_ms <= 0.0)
{
- pg_log_fatal("invalid latency limit: \"%s\"", optarg);
+ pg_log_fatal("-L/--latency-limit must be greater than zero: \"%s\"",
+ optarg);
exit(1);
}
+ /*
+ * limit_ms * 1000 must fit int64. We could calculate the
+ * precise limit but also we don't need to accept such a
+ * large number here. Thus use a quite arbitrary seconds
+ * for the limit.
+ */
+ if (limit_ms > 3600)
+ {
+ pg_log_fatal("-L/--latency-limit must be less than 3600 seconds: \"%s\"",
+ optarg);
+ exit(1);
+ }
+
benchmarking_option_set = true;
latency_limit = (int64) (limit_ms * 1000);
}
@@ -6089,21 +6101,21 @@ main(int argc, char **argv)
break;
case 4: /* sampling-rate */
benchmarking_option_set = true;
- sample_rate = atof(optarg);
+ if (option_parse_double(PG_LOG_FATAL, optarg, "--samplig-rate",
+ &sample_rate))
+ exit(1);
if (sample_rate <= 0.0 || sample_rate > 1.0)
{
- pg_log_fatal("invalid sampling rate: \"%s\"", optarg);
+ pg_log_fatal("sampling rate must be greater than zero and less than or equal to 1.0: \"%s\"", optarg);
exit(1);
}
break;
case 5: /* aggregate-interval */
benchmarking_option_set = true;
- agg_interval = atoi(optarg);
- if (agg_interval <= 0)
- {
- pg_log_fatal("invalid number of seconds for aggregation: \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_FATAL, optarg,
+ "--aggregate-interval",
+ 1, INT_MAX, &agg_interval))
exit(1);
- }
break;
case 6: /* progress-timestamp */
progress_timestamp = true;
@@ -6135,12 +6147,9 @@ main(int argc, char **argv)
break;
case 11: /* partitions */
initialization_option_set = true;
- partitions = atoi(optarg);
- if (partitions < 0)
- {
- pg_log_fatal("invalid number of partitions: \"%s\"", optarg);
+ if (option_parse_int(PG_LOG_FATAL, optarg, "--partitions",
+ 0, INT_MAX, &partitions))
exit(1);
- }
break;
case 12: /* partition-method */
initialization_option_set = true;
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 49d4c0e3ce..312a797e78 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -11,6 +11,7 @@
#include <time.h>
#include <pwd.h>
#include <utime.h>
+#include <limits.h>
#ifndef WIN32
#include <sys/stat.h> /* for stat() */
#include <sys/time.h> /* for setitimer() */
@@ -34,6 +35,7 @@
#include "describe.h"
#include "fe_utils/cancel.h"
#include "fe_utils/print.h"
+#include "fe_utils/option_utils.h"
#include "fe_utils/string_utils.h"
#include "help.h"
#include "input.h"
@@ -1040,11 +1042,19 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch,
}
if (ln)
{
- lineno = atoi(ln);
- if (lineno < 1)
+ switch (option_parse_int(PG_LOG_OMIT, ln, "line number",
+ 1, INT_MAX, &lineno))
{
- pg_log_error("invalid line number: %s", ln);
- status = PSQL_CMD_ERROR;
+ case OPTPARSE_SUCCESS:
+ break;
+ case OPTPARSE_MALFORMED:
+ case OPTPARSE_TOOSMALL:
+ case OPTPARSE_TOOLARGE:
+ pg_log_error("line number must be an integer greater than zero: %s", ln);
+ break;
+ case OPTPARSE_OUTOFRANGE:
+ pg_log_error("line number out of range: %s", ln);
+ break;
}
}
if (status != PSQL_CMD_ERROR)
@@ -4284,7 +4294,16 @@ 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);
+ {
+ int v;
+
+ if (option_parse_int(PG_LOG_OMIT, value, "border", 0, 65535, &v))
+ {
+ pg_log_error("\\pset: border must be an integer in range 0..65535: \"%s\"", value);
+ return false;
+ }
+ popt->topt.border = v;
+ }
}
/* set expanded/vertical mode */
@@ -4440,7 +4459,22 @@ 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);
+ {
+ switch (option_parse_int(PG_LOG_OMIT, value, "page_min_lines",
+ 0, INT_MAX, &popt->topt.pager_min_lines))
+ {
+ case OPTPARSE_SUCCESS:
+ break;
+ case OPTPARSE_MALFORMED:
+ case OPTPARSE_TOOSMALL:
+ case OPTPARSE_TOOLARGE:
+ pg_log_error("\\pset: pager_min_lines must be a non-negative integer: \"%s\"", value);
+ return false;
+ case OPTPARSE_OUTOFRANGE:
+ pg_log_error("\\pset: pager_min_lines out of range: \"%s\"", value);
+ return false;
+ }
+ }
}
/* disable "(x rows)" footer */
@@ -4456,7 +4490,22 @@ 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);
+ {
+ switch (option_parse_int(PG_LOG_OMIT, value, "columns",
+ 0, INT_MAX, &popt->topt.columns))
+ {
+ case OPTPARSE_SUCCESS:
+ break;
+ case OPTPARSE_MALFORMED:
+ case OPTPARSE_TOOSMALL:
+ case OPTPARSE_TOOLARGE:
+ pg_log_error("\\pset: column must be a non-negative integer: \"%s\"", value);
+ return false;
+ case OPTPARSE_OUTOFRANGE:
+ pg_log_error("\\pset: column out of range: \"%s\"", value);
+ return false;
+ }
+ }
}
else
{
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fc0681538a..f510a63157 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -9,6 +9,8 @@
*-------------------------------------------------------------------------
*/
+#include <limits.h>
+
#include "postgres_fe.h"
#include "catalog/pg_class_d.h"
@@ -151,12 +153,9 @@ main(int argc, char *argv[])
simple_string_list_append(&indexes, optarg);
break;
case 'j':
- concurrentCons = atoi(optarg);
- if (concurrentCons <= 0)
- {
- pg_log_error("number of parallel jobs must be at least 1");
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-j/--jobs",
+ 1, INT_MAX, &concurrentCons))
exit(1);
- }
break;
case 'v':
verbose = true;
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index a85919c5c1..21966ed7e2 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -10,6 +10,8 @@
*-------------------------------------------------------------------------
*/
+#include <limits.h>
+
#include "postgres_fe.h"
#include "catalog/pg_class_d.h"
@@ -17,6 +19,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"
@@ -192,20 +195,14 @@ main(int argc, char *argv[])
vacopts.verbose = true;
break;
case 'j':
- concurrentCons = atoi(optarg);
- if (concurrentCons <= 0)
- {
- pg_log_error("number of parallel jobs must be at least 1");
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-j/--jobs",
+ 1, INT_MAX, &concurrentCons))
exit(1);
- }
break;
case 'P':
- vacopts.parallel_workers = atoi(optarg);
- if (vacopts.parallel_workers < 0)
- {
- pg_log_error("parallel workers for vacuum must be greater than or equal to zero");
+ if (option_parse_int(PG_LOG_ERROR, optarg, "-P/--parallel",
+ 0, INT_MAX, &vacopts.parallel_workers))
exit(1);
- }
break;
case 2:
maintenance_db = pg_strdup(optarg);
@@ -220,20 +217,14 @@ main(int argc, char *argv[])
vacopts.skip_locked = true;
break;
case 6:
- vacopts.min_xid_age = atoi(optarg);
- if (vacopts.min_xid_age <= 0)
- {
- pg_log_error("minimum transaction ID age must be at least 1");
+ if (option_parse_int(PG_LOG_ERROR, optarg, "--min-xid-age",
+ 1, INT_MAX, &vacopts.min_xid_age))
exit(1);
- }
break;
case 7:
- vacopts.min_mxid_age = atoi(optarg);
- if (vacopts.min_mxid_age <= 0)
- {
- pg_log_error("minimum multixact ID age must be at least 1");
+ if (option_parse_int(PG_LOG_ERROR, optarg, "--min-mxid-age",
+ 1, INT_MAX, &vacopts.min_mxid_age))
exit(1);
- }
break;
case 8:
vacopts.no_index_cleanup = true;
diff --git a/src/fe_utils/option_utils.c b/src/fe_utils/option_utils.c
index e19a495dba..4f191d321a 100644
--- a/src/fe_utils/option_utils.c
+++ b/src/fe_utils/option_utils.c
@@ -10,8 +10,12 @@
*-------------------------------------------------------------------------
*/
+#include <limits.h>
+
#include "postgres_fe.h"
+#include "common/string.h"
+#include "common/logging.h"
#include "fe_utils/option_utils.h"
/*
@@ -36,3 +40,151 @@ handle_help_version_opts(int argc, char *argv[],
}
}
}
+
+/*
+ * parser for int/long values in decimal for command line options
+ *
+ * Returns OPTPARSE_SUCCESS (0) when successfully parsed.
+ *
+ * Otherwise returns non-zero status numbers indicating error details after
+ * printing a generic error message with the given errorlevel, which may be
+ * omitted when the errorlevel is lower than the common/logging.c's
+ * setting. Giving PG_LOG_OMIT as loglevel disables generic errors.
+ *
+ * If result is not NULL, the result is stored there only when successfully
+ * parsed.
+ */
+optparse_result
+option_parse_int(int loglevel, const char *optarg, const char *optname,
+ int min_range, int max_range, int *result)
+{
+ char *endptr;
+ int parsed;
+
+ errno = 0;
+ parsed = strtoint(optarg, &endptr, 10);
+
+ if (*endptr)
+ {
+ pg_log_level(loglevel, "%s must be an integer: \"%s\"",
+ optname, optarg);
+ return OPTPARSE_MALFORMED;
+ }
+ else if (errno == ERANGE || parsed < min_range || parsed > max_range)
+ {
+ if (max_range == INT_MAX)
+ {
+ if (errno == ERANGE)
+ pg_log_level(loglevel, "%s out of range: %s",
+ optname, optarg);
+ else
+ pg_log_level(loglevel, "%s must be at least %d: %s",
+ optname, min_range, optarg);
+ }
+ else
+ pg_log_level(loglevel, "%s must be in range %d..%d: %s",
+ optname, min_range, max_range, optarg);
+
+ if (errno == ERANGE)
+ return OPTPARSE_OUTOFRANGE;
+ if (parsed < min_range)
+ return OPTPARSE_TOOSMALL;
+
+ Assert (parsed > max_range);
+ return OPTPARSE_TOOLARGE;
+ }
+
+ if (result)
+ *result = parsed;
+
+ return OPTPARSE_SUCCESS;
+}
+
+
+optparse_result
+option_parse_long(int loglevel, const char *optarg, const char *optname,
+ long min_range, long max_range, long *result)
+{
+ char *endptr;
+ long parsed;
+
+ errno = 0;
+ parsed = strtol(optarg, &endptr, 10);
+
+ if (*endptr)
+ {
+ pg_log_level(loglevel, "%s must be an integer: \"%s\"",
+ optname, optarg);
+ return OPTPARSE_MALFORMED;
+ }
+ else if (errno == ERANGE || parsed < min_range || parsed > max_range)
+ {
+ if (max_range == LONG_MAX)
+ {
+ if (errno == ERANGE)
+ pg_log_level(loglevel, "%s out of range: %s",
+ optname, optarg);
+ else
+ pg_log_level(loglevel, "%s must be at least %ld: %s",
+ optname, min_range, optarg);
+ }
+ else
+ pg_log_level(loglevel, "%s must be in range %ld..%ld: %s",
+ optname, min_range, max_range, optarg);
+
+ if (errno == ERANGE)
+ return OPTPARSE_OUTOFRANGE;
+ if (parsed < min_range)
+ return OPTPARSE_TOOSMALL;
+
+ Assert (parsed > max_range);
+ return OPTPARSE_TOOLARGE;
+ }
+
+ if (result)
+ *result = parsed;
+
+ return OPTPARSE_SUCCESS;
+}
+
+
+/*
+ * parser for double value in decimal for command line options
+ *
+ * Returns OPTPARSE_SUCCESS (0) when successfully parsed.
+ *
+ * Otherwise returns non-zero status numbers indicating error details after
+ * printing a fixed-form error message with the given errorlevel, which may be
+ * omitted when the errorlevel is lower than the common/logging.c's
+ * setting. Giving PG_LOG_OMIT as loglevel disables this error output.
+ *
+ * If result is not NULL, the result is stored there only when successfully
+ * parsed.
+ */
+optparse_result
+option_parse_double(int loglevel, const char *optarg, const char *optname,
+ double *result)
+{
+ char *endptr;
+ double parsed;
+
+ errno = 0;
+ parsed = strtod(optarg, &endptr);
+
+ if (*endptr)
+ {
+ pg_log_level(loglevel, "%s must be a numeric: \"%s\"", optname, optarg);
+ return OPTPARSE_MALFORMED;
+ }
+ else if (errno == ERANGE)
+ {
+ pg_log_level(loglevel, "%s out of range: %s", optname, optarg);
+
+ return OPTPARSE_OUTOFRANGE;
+ }
+
+ if (result)
+ *result = parsed;
+
+ return OPTPARSE_SUCCESS;
+}
diff --git a/src/include/common/logging.h b/src/include/common/logging.h
index a71cf84249..c2e9d1e46f 100644
--- a/src/include/common/logging.h
+++ b/src/include/common/logging.h
@@ -15,6 +15,11 @@
*/
enum pg_log_level
{
+ /*
+ * Don't show this message
+ */
+ PG_LOG_OMIT = -1,
+
/*
* Not initialized yet
*/
@@ -93,4 +98,8 @@ void pg_log_generic_v(enum pg_log_level level, const char *pg_restrict fmt, va_
if (unlikely(__pg_log_level <= PG_LOG_DEBUG)) pg_log_generic(PG_LOG_DEBUG, __VA_ARGS__); \
} while(0)
+#define pg_log_level(loglevel, ...) do { \
+ if (unlikely(__pg_log_level <= loglevel)) pg_log_generic(loglevel, __VA_ARGS__); \
+ } while(0)
+
#endif /* COMMON_LOGGING_H */
diff --git a/src/include/fe_utils/option_utils.h b/src/include/fe_utils/option_utils.h
index d653cb94e3..bfba7a73ed 100644
--- a/src/include/fe_utils/option_utils.h
+++ b/src/include/fe_utils/option_utils.h
@@ -14,10 +14,33 @@
#include "postgres_fe.h"
+typedef enum optparse_result
+{
+ OPTPARSE_SUCCESS = 0,
+ OPTPARSE_MALFORMED,
+ OPTPARSE_TOOSMALL,
+ OPTPARSE_TOOLARGE,
+ OPTPARSE_OUTOFRANGE
+} optparse_result;
+
typedef void (*help_handler) (const char *progname);
extern void handle_help_version_opts(int argc, char *argv[],
const char *fixed_progname,
help_handler hlp);
+extern optparse_result option_parse_int(int loglevel,
+ const char *optarg, const char *optname,
+ int min_range, int max_range,
+ int *result);
+
+extern optparse_result option_parse_long(int loglevel,
+ const char *optarg,
+ const char *optname,
+ long min_range, long max_range,
+ long *result);
+
+extern optparse_result option_parse_double(int loglevel, const char *optarg,
+ const char *optname, double *result);
+
#endif /* OPTION_UTILS_H */
--
2.27.0
----Next_Part(Wed_Jul_21_17_02_29_2021_399)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v3-0002-Make-complain-for-invalid-numeirc-values-in-envir.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v3 1/3] Be strict in numeric parameters on command line
@ 2021-07-08 06:08 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Kyotaro Horiguchi @ 2021-07-08 06:08 UTC (permalink / raw)
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
----Next_Part(Wed_Jul_14_10_35_56_2021_265)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v3-0002-Make-complain-for-invalid-numeirc-values-in-envir.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH 1/2] Be strict in numeric parameters on command line
@ 2021-07-08 06:08 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Kyotaro Horiguchi @ 2021-07-08 06:08 UTC (permalink / raw)
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 | 6 ++-
src/bin/pg_basebackup/pg_basebackup.c | 13 +++--
src/bin/pg_basebackup/pg_receivewal.c | 18 +++++--
src/bin/pg_basebackup/pg_recvlogical.c | 17 +++++--
src/bin/pg_checksums/pg_checksums.c | 7 ++-
src/bin/pg_ctl/pg_ctl.c | 18 ++++++-
src/bin/pg_dump/pg_dump.c | 39 ++++++++-------
src/bin/pg_dump/pg_restore.c | 17 ++++---
src/bin/pg_upgrade/option.c | 21 ++++++--
src/bin/pgbench/pgbench.c | 66 ++++++++++++++++----------
src/bin/psql/command.c | 52 ++++++++++++++++++--
src/bin/scripts/reindexdb.c | 10 ++--
src/bin/scripts/vacuumdb.c | 23 +++++----
13 files changed, 219 insertions(+), 88 deletions(-)
diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 4bde16fb4b..71a82f9b75 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,8 +327,9 @@ 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 || errno == ERANGE || opts.jobs < 1)
{
pg_log_error("number of parallel jobs must be at least 1");
exit(1);
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8bb0acf498..29be95b96a 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,8 +2373,10 @@ 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);
exit(1);
@@ -2409,8 +2413,9 @@ 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 || errno == ERANGE || standby_message_timeout < 0)
{
pg_log_error("invalid status interval \"%s\"", optarg);
exit(1);
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index c1334fad35..7fef925b99 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,8 @@ 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;
+
switch (c)
{
case 'D':
@@ -532,7 +535,9 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
+ errno = 0;
+ if (strtoint(optarg, &endptr, 10) <= 0 ||
+ *endptr || errno == ERANGE)
{
pg_log_error("invalid port number \"%s\"", optarg);
exit(1);
@@ -549,8 +554,9 @@ 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 || errno == ERANGE || standby_message_timeout < 0)
{
pg_log_error("invalid status interval \"%s\"", optarg);
exit(1);
@@ -574,8 +580,10 @@ 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);
exit(1);
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 76bd153fac..7be932d025 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,8 @@ 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;
+
switch (c)
{
/* general options */
@@ -739,8 +742,9 @@ 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 || errno == ERANGE || fsync_interval < 0)
{
pg_log_error("invalid fsync interval \"%s\"", optarg);
exit(1);
@@ -763,7 +767,9 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
+ errno = 0;
+ if (strtoint(optarg, &endptr, 10) <= 0 ||
+ *endptr || errno == ERANGE)
{
pg_log_error("invalid port number \"%s\"", optarg);
exit(1);
@@ -820,8 +826,9 @@ 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 || errno == ERANGE || standby_message_timeout < 0)
{
pg_log_error("invalid status interval \"%s\"", optarg);
exit(1);
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 3c326906e2..1c4e5b9d85 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,8 @@ main(int argc, char *argv[])
while ((c = getopt_long(argc, argv, "cD:deNPf:v", long_options, &option_index)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'c':
@@ -518,7 +521,9 @@ main(int argc, char *argv[])
mode = PG_MODE_ENABLE;
break;
case 'f':
- if (atoi(optarg) == 0)
+ errno = 0;
+ if (strtoint(optarg, &endptr, 10) == 0
+ || *endptr || errno == ERANGE)
{
pg_log_error("invalid filenode specification, must be numeric: %s", optarg);
exit(1);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7985da0a94..78882dec48 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2331,6 +2331,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 +2398,13 @@ main(int argc, char **argv)
#endif
break;
case 't':
- wait_seconds = atoi(optarg);
+ errno = 0;
+ wait_seconds = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || wait_seconds < 0)
+ {
+ pg_log_error("invalid timeout \"%s\"", optarg);
+ exit(1);
+ }
wait_seconds_arg = true;
break;
case 'U':
@@ -2459,7 +2467,13 @@ 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 || errno == ERANGE || killproc < 0)
+ {
+ pg_log_error("invalid process ID \"%s\"", argv[optind]);
+ exit(1);
+ }
}
#ifdef WIN32
else if (strcmp(argv[optind], "register") == 0)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 321152151d..793f4b3509 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"
@@ -486,7 +487,19 @@ main(int argc, char **argv)
break;
case 'j': /* number of dump jobs */
- numWorkers = atoi(optarg);
+ errno = 0;
+ numWorkers = strtoint(optarg, &endptr, 10);
+ /*
+ * 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 (*endptr || errno == ERANGE || numWorkers <= 0
+#ifdef WIN32
+ || numWorkers > MAXIMUM_WAIT_OBJECTS
+#endif
+ )
+ fatal("invalid number of parallel jobs %s", optarg);
break;
case 'n': /* include schema(s) */
@@ -549,8 +562,10 @@ 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");
exit_nicely(1);
@@ -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);
@@ -719,18 +736,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..285a09aaac 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
@@ -151,6 +152,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 +184,13 @@ main(int argc, char **argv)
break;
case 'j': /* number of restore jobs */
- numWorkers = atoi(optarg);
+ errno = 0;
+ numWorkers = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || numWorkers <= 0)
+ {
+ pg_log_error("invalid number of parallel jobs");
+ exit(1);
+ }
break;
case 'l': /* Dump the TOC summary */
@@ -344,12 +353,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)
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 64bbda5650..357ba04f4f 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,12 @@ parseCommandLine(int argc, char *argv[])
break;
case 'j':
- user_opts.jobs = atoi(optarg);
+ errno = 0;
+ user_opts.jobs = strtoint(optarg, &endptr, 10);
+ /**/
+ if (*endptr || errno == ERANGE)
+ pg_fatal("invalid number of jobs %s\n", optarg);
+
break;
case 'k':
@@ -166,13 +173,17 @@ 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;
+ if ((old_cluster.port = strtoint(optarg, &endptr, 10)) <= 0 ||
+ *endptr || errno == ERANGE)
+ pg_fatal("invalid old port number %s\n", optarg);
break;
case 'P':
- if ((new_cluster.port = atoi(optarg)) <= 0)
- pg_fatal("invalid new port number\n");
+ errno = 0;
+ if ((new_cluster.port = strtoint(optarg, &endptr, 10)) <= 0 ||
+ *endptr || errno == ERANGE)
+ pg_fatal("invalid new port number %s\n", optarg);
break;
case 'r':
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 4aeccd93af..4020347585 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5838,6 +5838,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)
{
@@ -5869,8 +5870,9 @@ 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 || errno == ERANGE || nclients <= 0)
{
pg_log_fatal("invalid number of clients: \"%s\"", optarg);
exit(1);
@@ -5896,8 +5898,9 @@ 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 || errno == ERANGE || nthreads <= 0)
{
pg_log_fatal("invalid number of threads: \"%s\"", optarg);
exit(1);
@@ -5920,8 +5923,9 @@ 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 || errno == ERANGE || scale <= 0)
{
pg_log_fatal("invalid scaling factor: \"%s\"", optarg);
exit(1);
@@ -5929,8 +5933,9 @@ main(int argc, char **argv)
break;
case 't':
benchmarking_option_set = true;
- nxacts = atoi(optarg);
- if (nxacts <= 0)
+ errno = 0;
+ nxacts = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || nxacts <= 0)
{
pg_log_fatal("invalid number of transactions: \"%s\"", optarg);
exit(1);
@@ -5938,8 +5943,9 @@ main(int argc, char **argv)
break;
case 'T':
benchmarking_option_set = true;
- duration = atoi(optarg);
- if (duration <= 0)
+ errno = 0;
+ duration = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || duration <= 0)
{
pg_log_fatal("invalid duration: \"%s\"", optarg);
exit(1);
@@ -6001,8 +6007,10 @@ 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);
exit(1);
@@ -6021,8 +6029,9 @@ 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 || errno == ERANGE || progress <= 0)
{
pg_log_fatal("invalid thread progress delay: \"%s\"", optarg);
exit(1);
@@ -6031,11 +6040,13 @@ main(int argc, char **argv)
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 || errno == ERANGE || throttle_value <= 0.0)
{
pg_log_fatal("invalid rate limit: \"%s\"", optarg);
exit(1);
@@ -6046,9 +6057,12 @@ 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 || errno == ERANGE || limit_ms <= 0.0)
{
pg_log_fatal("invalid latency limit: \"%s\"", optarg);
exit(1);
@@ -6071,8 +6085,10 @@ 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);
exit(1);
@@ -6080,8 +6096,9 @@ main(int argc, char **argv)
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 || errno == ERANGE || agg_interval <= 0)
{
pg_log_fatal("invalid number of seconds for aggregation: \"%s\"", optarg);
exit(1);
@@ -6117,8 +6134,9 @@ 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 || errno == ERANGE || partitions < 0)
{
pg_log_fatal("invalid number of partitions: \"%s\"", optarg);
exit(1);
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 543401c6d6..aaed986ae1 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1039,8 +1039,11 @@ 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 || errno == ERANGE || lineno < 1)
{
pg_log_error("invalid line number: %s", ln);
status = PSQL_CMD_ERROR;
@@ -4283,7 +4286,21 @@ 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 || errno == ERANGE ||
+ new_value < 0 || new_value > 65535)
+ {
+ pg_log_error("\\pset: border is invalid or out of range");
+ return false;
+ }
+
+ popt->topt.border = new_value;
+ }
}
/* set expanded/vertical mode */
@@ -4439,7 +4456,20 @@ 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 || errno == ERANGE || new_value < 0)
+ {
+ pg_log_error("\\pset: pager_min_lines is invalid or out of range");
+ return false;
+ }
+
+ popt->topt.pager_min_lines = new_value;
+ }
}
/* disable "(x rows)" footer */
@@ -4455,7 +4485,19 @@ 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 || errno == ERANGE || new_value < 0)
+ {
+ pg_log_error("\\pset: column is invalid or out of range");
+ return false;
+ }
+ popt->topt.columns = new_value;
+ }
}
else
{
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fc0681538a..baa68d58d8 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,11 @@ 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 || 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);
}
break;
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 61974baa78..93b563998a 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,16 +195,18 @@ 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 || errno == ERANGE || concurrentCons <= 0)
{
pg_log_error("number of parallel jobs must be at least 1");
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 || errno == ERANGE || vacopts.parallel_workers < 0)
{
pg_log_error("parallel workers for vacuum must be greater than or equal to zero");
exit(1);
@@ -220,16 +225,18 @@ 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 || errno == ERANGE || vacopts.min_xid_age <= 0)
{
pg_log_error("minimum transaction ID age must be at least 1");
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 || errno == ERANGE || vacopts.min_mxid_age <= 0)
{
pg_log_error("minimum multixact ID age must be at least 1");
exit(1);
--
2.27.0
----Next_Part(Thu_Jul__8_17_30_23_2021_499)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Make-complain-for-invalid-numeirc-values-in-environe.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pg_stats and range statistics
@ 2023-11-25 08:57 jian he <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: jian he @ 2023-11-25 08:57 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Gregory Stark (as CFM) <[email protected]>; Egor Rogov <[email protected]>; Tomas Vondra <[email protected]>; Justin Pryzby <[email protected]>; Soumyadeep Chakraborty <[email protected]>; [email protected]
On Sat, Nov 25, 2023 at 7:06 AM Alexander Korotkov <[email protected]> wrote:
>
> Hi!
> Additionally, I found that the current patch can't handle infinite
> range bounds and discards information about inclusiveness of range
> bounds. The infinite bounds could be represented as NULL (while I'm
> not sure how good this representation is). Regarding inclusiveness, I
> don't see the possibility to represent them in a reasonable way within
> an array of base types. I also don't feel good about discarding the
> accuracy in the pg_stats view.
>
in range_length_histogram, maybe we can document that when calculating
the length of a range, inclusiveness will be true.
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pg_stats and range statistics
@ 2023-11-25 16:55 Alexander Korotkov <[email protected]>
parent: jian he <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Alexander Korotkov @ 2023-11-25 16:55 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: Gregory Stark (as CFM) <[email protected]>; Egor Rogov <[email protected]>; Tomas Vondra <[email protected]>; Justin Pryzby <[email protected]>; Soumyadeep Chakraborty <[email protected]>; [email protected]
On Sat, Nov 25, 2023 at 10:58 AM jian he <[email protected]> wrote:
> On Sat, Nov 25, 2023 at 7:06 AM Alexander Korotkov <[email protected]> wrote:
> >
> > Hi!
> > Additionally, I found that the current patch can't handle infinite
> > range bounds and discards information about inclusiveness of range
> > bounds. The infinite bounds could be represented as NULL (while I'm
> > not sure how good this representation is). Regarding inclusiveness, I
> > don't see the possibility to represent them in a reasonable way within
> > an array of base types. I also don't feel good about discarding the
> > accuracy in the pg_stats view.
> >
>
> in range_length_histogram, maybe we can document that when calculating
> the length of a range, inclusiveness will be true.
I've revised the patchset. Edited comment in pg_statistic.h as you
proposed. And I've added to the documentation a short note on how the
range length histogram is calculated.
------
Regards,
Alexander Korotkov
Attachments:
[application/octet-stream] 0001-Update-comments-for-pg_statistic-catalog--20231125-2.patch (1.9K, ../../CAPpHfdvu3w-MWcSXPPrBx1z5QTjSV-=Jm3xueAGzwjB7bLyvpQ@mail.gmail.com/2-0001-Update-comments-for-pg_statistic-catalog--20231125-2.patch)
download | inline diff:
From 1887f2ed8cc16a00c60bf72533a8bb195c040faf Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 25 Nov 2023 00:38:05 +0200
Subject: [PATCH 1/2] Update comments for pg_statistic catalog table
Make a reminder that pg_stats view needs to be modified whenever a new slot
kind is added. To prevent situations like 918eee0c49 when pg_stats was
forgotten to be updated.
Also, revise the comment that only non-null, non-empty rows are considered
for the range length histogram.
Discussion: https://postgr.es/m/flat/[email protected]
Author: Egor Rogov, Soumyadeep Chakraborty
Reviewed-by: Tomas Vondra, Justin Pryzby, Jian He
---
src/include/catalog/pg_statistic.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/include/catalog/pg_statistic.h b/src/include/catalog/pg_statistic.h
index 0808c5fc893..87f48dc6894 100644
--- a/src/include/catalog/pg_statistic.h
+++ b/src/include/catalog/pg_statistic.h
@@ -152,6 +152,9 @@ DECLARE_FOREIGN_KEY((starelid, staattnum), pg_attribute, (attrelid, attnum));
* data "kind" will appear in any particular slot. Instead, search the
* stakind fields to see if the desired data is available. (The standard
* function get_attstatsslot() may be used for this.)
+ *
+ * Note: The pg_stats view needs to be modified whenever a new slot kind is
+ * added to core.
*/
/*
@@ -262,7 +265,8 @@ DECLARE_FOREIGN_KEY((starelid, staattnum), pg_attribute, (attrelid, attnum));
* a format similar to STATISTIC_KIND_HISTOGRAM: it contains M (>=2) range
* values that divide the column data values into M-1 bins of approximately
* equal population. The lengths are stored as float8s, as measured by the
- * range type's subdiff function. Only non-null rows are considered.
+ * range type's subdiff function. Only non-null, non-empty rows are
+ * considered.
*/
#define STATISTIC_KIND_RANGE_LENGTH_HISTOGRAM 6
--
2.39.3 (Apple Git-145)
[application/octet-stream] 0002-Display-length-and-bounds-histograms-in-p-20231125-2.patch (6.0K, ../../CAPpHfdvu3w-MWcSXPPrBx1z5QTjSV-=Jm3xueAGzwjB7bLyvpQ@mail.gmail.com/3-0002-Display-length-and-bounds-histograms-in-p-20231125-2.patch)
download | inline diff:
From 235b2f3de5fff7ae07b0499831435d7b536d6306 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 25 Nov 2023 00:38:37 +0200
Subject: [PATCH 2/2] Display length and bounds histograms in pg_stats
Values corresponding to STATISTIC_KIND_RANGE_LENGTH_HISTOGRAM and
STATISTIC_KIND_BOUNDS_HISTOGRAM were not exposed to pg_stats when these
slot kinds were introduced in 918eee0c49.
This commit adds the missing fields to pg_stats.
Catversion is bumped.
Discussion: https://postgr.es/m/flat/[email protected]
Author: Egor Rogov, Soumyadeep Chakraborty
Reviewed-by: Tomas Vondra, Justin Pryzby, Jian He
---
doc/src/sgml/system-views.sgml | 39 ++++++++++++++++++++++++++++
src/backend/catalog/system_views.sql | 23 +++++++++++++++-
src/test/regress/expected/rules.out | 26 ++++++++++++++++++-
3 files changed, 86 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 7078491c4c0..0ef17456318 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -3799,6 +3799,45 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
non-null elements. (Null for scalar types.)
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>range_length_histogram</structfield> <type>anyarray</type>
+ </para>
+ <para>
+ A histogram of the lengths of non-empty and non-null range values of a
+ range type column. (Null for non-range types.)
+ </para>
+ <para>
+ This histogram is calculated using the <literal>subtype_diff</literal>
+ range function regardless of whether range bounds are inclusive.
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>range_empty_frac</structfield> <type>float4</type>
+ </para>
+ <para>
+ Fraction of column entries whose values are empty ranges.
+ (Null for non-range types.)
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>range_bounds_histogram</structfield> <type>anyarray</type>
+ </para>
+ <para>
+ A histogram of lower and upper bounds of non-empty and non-null range
+ values. (Null for non-range types.)
+ </para>
+ <para>
+ These two histograms are represented as a single array of ranges, whose
+ lower bounds represent the histogram of lower bounds, and upper bounds
+ represent the histogram of upper bounds.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index b65f6b52490..11d18ed9dd6 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -243,7 +243,28 @@ CREATE VIEW pg_stats WITH (security_barrier) AS
WHEN stakind3 = 5 THEN stanumbers3
WHEN stakind4 = 5 THEN stanumbers4
WHEN stakind5 = 5 THEN stanumbers5
- END AS elem_count_histogram
+ END AS elem_count_histogram,
+ CASE
+ WHEN stakind1 = 6 THEN stavalues1
+ WHEN stakind2 = 6 THEN stavalues2
+ WHEN stakind3 = 6 THEN stavalues3
+ WHEN stakind4 = 6 THEN stavalues4
+ WHEN stakind5 = 6 THEN stavalues5
+ END AS range_length_histogram,
+ CASE
+ WHEN stakind1 = 6 THEN stanumbers1[1]
+ WHEN stakind2 = 6 THEN stanumbers2[1]
+ WHEN stakind3 = 6 THEN stanumbers3[1]
+ WHEN stakind4 = 6 THEN stanumbers4[1]
+ WHEN stakind5 = 6 THEN stanumbers5[1]
+ END AS range_empty_frac,
+ CASE
+ WHEN stakind1 = 7 THEN stavalues1
+ WHEN stakind2 = 7 THEN stavalues2
+ WHEN stakind3 = 7 THEN stavalues3
+ WHEN stakind4 = 7 THEN stavalues4
+ WHEN stakind5 = 7 THEN stavalues5
+ END AS range_bounds_histogram
FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid)
JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum)
LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace)
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 1442c43d9c5..05070393b99 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2466,7 +2466,31 @@ pg_stats| SELECT n.nspname AS schemaname,
WHEN (s.stakind4 = 5) THEN s.stanumbers4
WHEN (s.stakind5 = 5) THEN s.stanumbers5
ELSE NULL::real[]
- END AS elem_count_histogram
+ END AS elem_count_histogram,
+ CASE
+ WHEN (s.stakind1 = 6) THEN s.stavalues1
+ WHEN (s.stakind2 = 6) THEN s.stavalues2
+ WHEN (s.stakind3 = 6) THEN s.stavalues3
+ WHEN (s.stakind4 = 6) THEN s.stavalues4
+ WHEN (s.stakind5 = 6) THEN s.stavalues5
+ ELSE NULL::anyarray
+ END AS range_length_histogram,
+ CASE
+ WHEN (s.stakind1 = 6) THEN s.stanumbers1[1]
+ WHEN (s.stakind2 = 6) THEN s.stanumbers2[1]
+ WHEN (s.stakind3 = 6) THEN s.stanumbers3[1]
+ WHEN (s.stakind4 = 6) THEN s.stanumbers4[1]
+ WHEN (s.stakind5 = 6) THEN s.stanumbers5[1]
+ ELSE NULL::real
+ END AS range_empty_frac,
+ CASE
+ WHEN (s.stakind1 = 7) THEN s.stavalues1
+ WHEN (s.stakind2 = 7) THEN s.stavalues2
+ WHEN (s.stakind3 = 7) THEN s.stavalues3
+ WHEN (s.stakind4 = 7) THEN s.stavalues4
+ WHEN (s.stakind5 = 7) THEN s.stavalues5
+ ELSE NULL::anyarray
+ END AS range_bounds_histogram
FROM (((pg_statistic s
JOIN pg_class c ON ((c.oid = s.starelid)))
JOIN pg_attribute a ON (((c.oid = a.attrelid) AND (a.attnum = s.staattnum))))
--
2.39.3 (Apple Git-145)
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2023-11-25 16:55 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-11-18 21:31 Re: [HACKERS] Repetitive code in RI triggers Tom Lane <[email protected]>
2017-11-20 08:53 ` Ildar Musin <[email protected]>
2021-07-08 06:08 [PATCH 1/2] Be strict in numeric parameters on command line Kyotaro Horiguchi <[email protected]>
2021-07-08 06:08 [PATCH v3 1/3] Be strict in numeric parameters on command line Kyotaro Horiguchi <[email protected]>
2021-07-08 06:08 [PATCH v3 1/3] Be strict in numeric parameters on command line Kyotaro Horiguchi <[email protected]>
2021-07-08 06:08 [PATCH v2 1/2] Be strict in numeric parameters on command line Kyotaro Horiguchi <[email protected]>
2023-11-25 08:57 Re: pg_stats and range statistics jian he <[email protected]>
2023-11-25 16:55 ` Re: pg_stats and range statistics Alexander Korotkov <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox