On Thu, Jul 22, 2021 at 01:19:41AM +1200, David Rowley wrote:
> On Thu, 22 Jul 2021 at 00:44, Michael Paquier <mich...@paquier.xyz> wrote:
>>
>> On Thu, Jul 22, 2021 at 12:32:39AM +1200, David Rowley wrote:
>> > I see both of these are limited to 64 on windows. Won't those fail on
>> > Windows?
>>
>> Yes, thanks, they would. I would just cut the range numbers from the
>> expected output here. This does not matter in terms of coverage
>> either.
>
> Sounds good.
>
>> x> I also wondered if it would be worth doing #define MAX_JOBS somewhere
>> > away from the option parsing code. This part is pretty ugly:
>>
>> Agreed as well. pg_dump and pg_restore have their own idea of
>> parallelism in parallel.{c.h}. What about putting MAX_JOBS in
>> parallel.h then?
>
> parallel.h looks ok to me.Okay, done those parts as per the attached. While on it, I noticed an extra one for pg_dump --rows-per-insert. I am counting 25 translated strings cut in total. Any objections to this first step? -- Michael
From 36be37c0dd0810b65d75e9807b61c244d4f56333 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 22 Jul 2021 14:31:31 +0900
Subject: [PATCH v5] Introduce and use routine for parsing of int32 options
---
src/include/fe_utils/option_utils.h | 3 ++
src/fe_utils/option_utils.c | 39 ++++++++++++++
src/bin/pg_amcheck/pg_amcheck.c | 8 ++-
src/bin/pg_basebackup/pg_basebackup.c | 17 +++---
src/bin/pg_basebackup/pg_receivewal.c | 23 +++------
src/bin/pg_basebackup/pg_recvlogical.c | 25 ++++-----
src/bin/pg_checksums/Makefile | 3 ++
src/bin/pg_checksums/pg_checksums.c | 9 ++--
src/bin/pg_dump/parallel.h | 13 +++++
src/bin/pg_dump/pg_dump.c | 46 ++++-------------
src/bin/pg_dump/pg_restore.c | 22 ++------
src/bin/pg_dump/t/001_basic.pl | 20 ++++----
src/bin/pgbench/pgbench.c | 60 +++++++---------------
src/bin/pgbench/t/002_pgbench_no_server.pl | 36 ++++++++-----
src/bin/scripts/reindexdb.c | 9 ++--
src/bin/scripts/vacuumdb.c | 31 ++++-------
16 files changed, 174 insertions(+), 190 deletions(-)
diff --git a/src/include/fe_utils/option_utils.h b/src/include/fe_utils/option_utils.h
index d653cb94e3..e999d56ec0 100644
--- a/src/include/fe_utils/option_utils.h
+++ b/src/include/fe_utils/option_utils.h
@@ -19,5 +19,8 @@ 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 bool option_parse_int(const char *optarg, const char *optname,
+ int min_range, int max_range,
+ int *result);
#endif /* OPTION_UTILS_H */
diff --git a/src/fe_utils/option_utils.c b/src/fe_utils/option_utils.c
index e19a495dba..a09d57db71 100644
--- a/src/fe_utils/option_utils.c
+++ b/src/fe_utils/option_utils.c
@@ -12,6 +12,8 @@
#include "postgres_fe.h"
+#include "common/logging.h"
+#include "common/string.h"
#include "fe_utils/option_utils.h"
/*
@@ -36,3 +38,40 @@ handle_help_version_opts(int argc, char *argv[],
}
}
}
+
+/*
+ * option_parse_int
+ *
+ * Parse integer value for an option. Returns true if the parsing could
+ * be done with optionally *result holding the parsed value, and false on
+ * failure.
+ */
+bool
+option_parse_int(const char *optarg, const char *optname,
+ int min_range, int max_range,
+ int *result)
+{
+ char *endptr;
+ int val;
+
+ errno = 0;
+ val = strtoint(optarg, &endptr, 10);
+
+ if (*endptr)
+ {
+ pg_log_error("invalid value \"%s\" for option \"%s\"",
+ optarg, optname);
+ return false;
+ }
+
+ if (errno == ERANGE || val < min_range || val > max_range)
+ {
+ pg_log_error("\"%s\" must be in range %d..%d",
+ optname, min_range, max_range);
+ return false;
+ }
+
+ if (result)
+ *result = val;
+ return true;
+}
diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 4bde16fb4b..ee8aa71bdf 100644
--- a/src/bin/pg_amcheck/pg_amcheck.c
+++ b/src/bin/pg_amcheck/pg_amcheck.c
@@ -12,6 +12,7 @@
*/
#include "postgres_fe.h"
+#include <limits.h>
#include <time.h>
#include "catalog/pg_am_d.h"
@@ -326,12 +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)
- {
- pg_log_error("number of parallel jobs must be at least 1");
+ if (!option_parse_int(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..9ea98481d8 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -31,6 +31,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 +2372,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(optarg, "-Z/--compress", 0, 9,
+ &compresslevel))
exit(1);
- }
break;
case 'c':
if (pg_strcasecmp(optarg, "fast") == 0)
@@ -2409,12 +2407,11 @@ 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(optarg, "-s/--status-interval", 0,
+ INT_MAX / 1000,
+ &standby_message_timeout))
exit(1);
- }
+ standby_message_timeout *= 1000;
break;
case 'v':
verbose++;
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index c1334fad35..4474273daf 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,7 @@
#include "access/xlog_internal.h"
#include "common/file_perm.h"
#include "common/logging.h"
+#include "fe_utils/option_utils.h"
#include "getopt_long.h"
#include "libpq-fe.h"
#include "receivelog.h"
@@ -532,11 +534,6 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
- {
- pg_log_error("invalid port number \"%s\"", optarg);
- exit(1);
- }
dbport = pg_strdup(optarg);
break;
case 'U':
@@ -549,12 +546,11 @@ 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(optarg, "-s/--status-interval", 0,
+ INT_MAX / 1000,
+ &standby_message_timeout))
exit(1);
- }
+ standby_message_timeout *= 1000;
break;
case 'S':
replication_slot = pg_strdup(optarg);
@@ -574,12 +570,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(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..1d59bf3744 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,7 @@
#include "common/fe_memutils.h"
#include "common/file_perm.h"
#include "common/logging.h"
+#include "fe_utils/option_utils.h"
#include "getopt_long.h"
#include "libpq-fe.h"
#include "libpq/pqsignal.h"
@@ -739,12 +741,11 @@ 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(optarg, "-F/--fsync-interval", 0,
+ INT_MAX / 1000,
+ &fsync_interval))
exit(1);
- }
+ fsync_interval *= 1000;
break;
case 'n':
noloop = 1;
@@ -763,11 +764,6 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
- {
- pg_log_error("invalid port number \"%s\"", optarg);
- exit(1);
- }
dbport = pg_strdup(optarg);
break;
case 'U':
@@ -820,12 +816,11 @@ 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(optarg, "-s/--status-interval", 0,
+ INT_MAX / 1000,
+ &standby_message_timeout))
exit(1);
- }
+ 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..4d1f0fc3a0 100644
--- a/src/bin/pg_checksums/Makefile
+++ b/src/bin/pg_checksums/Makefile
@@ -15,6 +15,9 @@ subdir = src/bin/pg_checksums
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
+# We need libpq only because fe_utils does.
+LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
+
OBJS = \
$(WIN32RES) \
pg_checksums.o
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 3c326906e2..e833c5d75e 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,7 @@
#include "common/file_perm.h"
#include "common/file_utils.h"
#include "common/logging.h"
+#include "fe_utils/option_utils.h"
#include "getopt_long.h"
#include "pg_getopt.h"
#include "storage/bufpage.h"
@@ -518,11 +520,10 @@ 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(optarg, "-f/--filenode", 0,
+ INT_MAX,
+ NULL))
exit(1);
- }
only_filenode = pstrdup(optarg);
break;
case 'N':
diff --git a/src/bin/pg_dump/parallel.h b/src/bin/pg_dump/parallel.h
index 0fbf736c81..0b39285a01 100644
--- a/src/bin/pg_dump/parallel.h
+++ b/src/bin/pg_dump/parallel.h
@@ -33,6 +33,19 @@ typedef enum
WFW_ALL_IDLE
} WFW_WaitOption;
+/*
+ * Maximum number of parallel jobs allowed.
+ *
+ * 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.
+ */
+#ifdef WIN32
+#define PG_MAX_JOBS MAXIMUM_WAIT_OBJECTS
+#else
+#define PG_MAX_JOBS INT_MAX
+#endif
+
/* ParallelSlot is an opaque struct known only within parallel.c */
typedef struct ParallelSlot ParallelSlot;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 34b91bb226..4251f5809e 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -56,6 +56,7 @@
#include "catalog/pg_type_d.h"
#include "common/connect.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"
@@ -322,14 +323,12 @@ main(int argc, char **argv)
DumpableObject *boundaryObjs;
int i;
int optindex;
- char *endptr;
RestoreOptions *ropt;
Archive *fout; /* the script file */
bool g_verbose = false;
const char *dumpencoding = NULL;
const char *dumpsnapshot = NULL;
char *use_role = NULL;
- long rowsPerInsert;
int numWorkers = 1;
int compressLevel = -1;
int plainText = 0;
@@ -487,7 +486,10 @@ main(int argc, char **argv)
break;
case 'j': /* number of dump jobs */
- numWorkers = atoi(optarg);
+ if (!option_parse_int(optarg, "-j/--jobs", 0,
+ PG_MAX_JOBS,
+ &numWorkers))
+ exit_nicely(1);
break;
case 'n': /* include schema(s) */
@@ -550,12 +552,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(optarg, "-Z/--compress", 0, 9,
+ &compressLevel))
exit_nicely(1);
- }
break;
case 0:
@@ -588,12 +587,9 @@ 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(optarg, "--extra-float-digits", -15, 3,
+ &extra_float_digits))
exit_nicely(1);
- }
break;
case 9: /* inserts */
@@ -608,17 +604,9 @@ main(int argc, char **argv)
case 10: /* rows per insert */
errno = 0;
- rowsPerInsert = strtol(optarg, &endptr, 10);
-
- if (endptr == optarg || *endptr != '\0' ||
- rowsPerInsert <= 0 || rowsPerInsert > INT_MAX ||
- errno == ERANGE)
- {
- pg_log_error("rows-per-insert must be in range %d..%d",
- 1, INT_MAX);
+ if (!option_parse_int(optarg, "--rows-per-insert", 1, INT_MAX,
+ &dopt.dump_inserts))
exit_nicely(1);
- }
- dopt.dump_inserts = (int) rowsPerInsert;
break;
case 11: /* include foreign data */
@@ -720,18 +708,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..184aa81c87 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -46,6 +46,7 @@
#endif
#include "dumputils.h"
+#include "fe_utils/option_utils.h"
#include "getopt_long.h"
#include "parallel.h"
#include "pg_backup_utils.h"
@@ -181,7 +182,10 @@ main(int argc, char **argv)
break;
case 'j': /* number of restore jobs */
- numWorkers = atoi(optarg);
+ if (!option_parse_int(optarg, "-j/--jobs", 0,
+ PG_MAX_JOBS,
+ &numWorkers))
+ exit(1);
break;
case 'l': /* Dump the TOC summary */
@@ -344,22 +348,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_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 9f12ca6c51..82b9c3d3ba 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -103,8 +103,8 @@ command_fails_like(
command_fails_like(
[ 'pg_dump', '-j', '-1' ],
- qr/\Qpg_dump: error: invalid number of parallel jobs\E/,
- 'pg_dump: invalid number of parallel jobs');
+ qr/\Qpg_dump: error: "-j\/--jobs" must be in range\E/,
+ 'pg_dump: "-j/--jobs" must be in range');
command_fails_like(
[ 'pg_dump', '-F', 'garbage' ],
@@ -113,8 +113,8 @@ command_fails_like(
command_fails_like(
[ 'pg_restore', '-j', '-1', '-f -' ],
- qr/\Qpg_restore: error: invalid number of parallel jobs\E/,
- 'pg_restore: invalid number of parallel jobs');
+ qr/\Qpg_restore: error: "-j\/--jobs" must be in range\E/,
+ 'pg_restore: "-j/--jobs" must be in range');
command_fails_like(
[ 'pg_restore', '--single-transaction', '-j3', '-f -' ],
@@ -123,18 +123,18 @@ command_fails_like(
command_fails_like(
[ 'pg_dump', '-Z', '-1' ],
- qr/\Qpg_dump: error: compression level must be in range 0..9\E/,
- 'pg_dump: compression level must be in range 0..9');
+ qr/\Qpg_dump: error: "-Z\/--compress" must be in range 0..9\E/,
+ 'pg_dump: "-Z/--compress" must be in range');
command_fails_like(
[ 'pg_dump', '--extra-float-digits', '-16' ],
- qr/\Qpg_dump: error: extra_float_digits must be in range -15..3\E/,
- 'pg_dump: extra_float_digits must be in range -15..3');
+ qr/\Qpg_dump: error: "--extra-float-digits" must be in range\E/,
+ 'pg_dump: "--extra-float-digits" must be in range');
command_fails_like(
[ 'pg_dump', '--rows-per-insert', '0' ],
- qr/\Qpg_dump: error: rows-per-insert must be in range 1..2147483647\E/,
- 'pg_dump: rows-per-insert must be in range 1..2147483647');
+ qr/\Qpg_dump: error: "--rows-per-insert" must be in range\E/,
+ 'pg_dump: "--rows-per-insert" must be in range');
command_fails_like(
[ 'pg_restore', '--if-exists', '-f -' ],
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 364b5a2e47..c51ebb8e31 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"
@@ -5887,10 +5888,9 @@ main(int argc, char **argv)
break;
case 'c':
benchmarking_option_set = true;
- nclients = atoi(optarg);
- if (nclients <= 0)
+ if (!option_parse_int(optarg, "-c/--clients", 1, INT_MAX,
+ &nclients))
{
- pg_log_fatal("invalid number of clients: \"%s\"", optarg);
exit(1);
}
#ifdef HAVE_GETRLIMIT
@@ -5914,10 +5914,9 @@ main(int argc, char **argv)
break;
case 'j': /* jobs */
benchmarking_option_set = true;
- nthreads = atoi(optarg);
- if (nthreads <= 0)
+ if (!option_parse_int(optarg, "-j/--jobs", 1, INT_MAX,
+ &nthreads))
{
- pg_log_fatal("invalid number of threads: \"%s\"", optarg);
exit(1);
}
#ifndef ENABLE_THREAD_SAFETY
@@ -5938,30 +5937,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(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(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(optarg, "-T/--time", 1, INT_MAX,
+ &duration))
exit(1);
- }
break;
case 'U':
username = pg_strdup(optarg);
@@ -6019,12 +6009,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(optarg, "-F/--fillfactor", 10, 100,
+ &fillfactor))
exit(1);
- }
break;
case 'M':
benchmarking_option_set = true;
@@ -6039,12 +6026,9 @@ 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(optarg, "-P/--progress", 1, INT_MAX,
+ &progress))
exit(1);
- }
break;
case 'R':
{
@@ -6098,12 +6082,9 @@ main(int argc, char **argv)
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(optarg, "--aggregate-interval", 1, INT_MAX,
+ &agg_interval))
exit(1);
- }
break;
case 6: /* progress-timestamp */
progress_timestamp = true;
@@ -6135,12 +6116,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(optarg, "--partitions", 1, INT_MAX,
+ &partitions))
exit(1);
- }
break;
case 12: /* partition-method */
initialization_option_set = true;
diff --git a/src/bin/pgbench/t/002_pgbench_no_server.pl b/src/bin/pgbench/t/002_pgbench_no_server.pl
index 346a2667fc..88eb311753 100644
--- a/src/bin/pgbench/t/002_pgbench_no_server.pl
+++ b/src/bin/pgbench/t/002_pgbench_no_server.pl
@@ -37,7 +37,7 @@ sub pgbench_scripts
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($opts, $stat, $out, $err, $name, $files) = @_;
- my @cmd = ('pgbench', split /\s+/, $opts);
+ my @cmd = ('pgbench', split /\s+/, $opts);
my @filenames = ();
if (defined $files)
{
@@ -91,17 +91,26 @@ my @options = (
[qr{weight spec.* out of range .*: -1}]
],
[ 'too many scripts', '-S ' x 129, [qr{at most 128 SQL scripts}] ],
- [ 'bad #clients', '-c three', [qr{invalid number of clients: "three"}] ],
[
- 'bad #threads', '-j eleven', [qr{invalid number of threads: "eleven"}]
+ 'bad #clients', '-c three',
+ [qr{invalid value "three" for option "-c/--clients"}]
+ ],
+ [
+ 'bad #threads', '-j eleven',
+ [qr{invalid value "eleven" for option "-j/--jobs"}]
+ ],
+ [
+ 'bad scale', '-i -s two',
+ [qr{invalid value "two" for option "-s/--scale"}]
],
- [ 'bad scale', '-i -s two', [qr{invalid scaling factor: "two"}] ],
[
'invalid #transactions',
- '-t zil',
- [qr{invalid number of transactions: "zil"}]
+ '-t zil', [qr{invalid value "zil" for option "-t/--transactions"}]
+ ],
+ [
+ 'invalid duration',
+ '-T ten', [qr{invalid value "ten" for option "-T/--time"}]
],
- [ 'invalid duration', '-T ten', [qr{invalid duration: "ten"}] ],
[
'-t XOR -T',
'-N -l --aggregate-interval=5 --log-prefix=notused -t 1000 -T 1',
@@ -113,11 +122,13 @@ my @options = (
[qr{specify either }]
],
[ 'bad variable', '--define foobla', [qr{invalid variable definition}] ],
- [ 'invalid fillfactor', '-F 1', [qr{invalid fillfactor}] ],
+ [
+ 'invalid fillfactor', '-F 1', [qr{"-F/--fillfactor" must be in range}]
+ ],
[ 'invalid query mode', '-M no-such-mode', [qr{invalid query mode}] ],
[
'invalid progress', '--progress=0',
- [qr{invalid thread progress delay}]
+ [qr{"-P/--progress" must be in range}]
],
[ 'invalid rate', '--rate=0.0', [qr{invalid rate limit}] ],
[ 'invalid latency', '--latency-limit=0.0', [qr{invalid latency limit}] ],
@@ -126,8 +137,9 @@ my @options = (
[qr{invalid sampling rate}]
],
[
- 'invalid aggregate interval', '--aggregate-interval=-3',
- [qr{invalid .* seconds for}]
+ 'invalid aggregate interval',
+ '--aggregate-interval=-3',
+ [qr{"--aggregate-interval" must be in range}]
],
[
'weight zero',
@@ -171,7 +183,7 @@ my @options = (
[
'bad partition number',
'-i --partitions -1',
- [qr{invalid number of partitions: "-1"}]
+ [qr{"--partitions" must be in range}]
],
[
'partition method without partitioning',
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fc0681538a..a0b0250c49 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -11,6 +11,8 @@
#include "postgres_fe.h"
+#include <limits.h>
+
#include "catalog/pg_class_d.h"
#include "common.h"
#include "common/connect.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(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..f40bd14857 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -12,8 +12,9 @@
#include "postgres_fe.h"
-#include "catalog/pg_class_d.h"
+#include <limits.h>
+#include "catalog/pg_class_d.h"
#include "common.h"
#include "common/connect.h"
#include "common/logging.h"
@@ -192,20 +193,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(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(optarg, "-P/--parallel", 0, INT_MAX,
+ &vacopts.parallel_workers))
exit(1);
- }
break;
case 2:
maintenance_db = pg_strdup(optarg);
@@ -220,20 +215,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(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(optarg, "--min-mxid-age", 1, INT_MAX,
+ &vacopts.min_mxid_age))
exit(1);
- }
break;
case 8:
vacopts.no_index_cleanup = true;
--
2.32.0
signature.asc
Description: PGP signature
