Re: Change atoi to strtol in same place - Mailing list pgsql-hackers
From | Joe Nelson |
---|---|
Subject | Re: Change atoi to strtol in same place |
Date | |
Msg-id | 20191007002150.GD68117@begriffs.com Whole thread Raw |
In response to | Re: Change atoi to strtol in same place (Ashutosh Sharma <ashu.coek88@gmail.com>) |
Responses |
Re: Change atoi to strtol in same place
|
List | pgsql-hackers |
Ashutosh Sharma wrote: > One suggestion: The start value for port number is set to 1, however > it seems like the port number that falls in the range of 1-1023 is > reserved and can't be used. So, is it possible to have the start value > as 1024 instead of 1 ? Good idea -- changed it. I also created macros FE_UTILS_PORT_{MIN,MAX} so the range can be updated in one place for all utilities. > Further, I encountered one syntax error (INT_MAX undeclared) as the > header file "limits.h" has not been included in postgres_fe.h or > option.h Oops. Added limits.h now in option.h. The Postgres build accidentally worked on my system without explicitly including this header because __has_builtin(__builtin_isinf) is true for me so src/include/port.h pulled in math.h with an #if which pulled in limits.h. > Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > > The wording is a bit strange. How about something like pg_standy: > > invalid argument to -k: %s I updated the error messages in pg_standby. > > > *error = psprintf(_("could not parse '%s' as integer"), str); > > > > ... except that they would rather be more explicit about what the > > problem is. "insufficient digits" or "extraneous character", etc. Sadly pg_strtoint64 returns the same error code for both cases. So we could either petition for more detailed errors in the thread for that other patch, or examine the string ourselves to check. Maybe it's not needed since "could not parse 'abc' as integer" kind of does show the problem. > > I hope that no callers would like to have the messages not translated, > > because that seems like it would become a mess. That's true... I think it should be OK though, since we return the pg_strtoint_status so callers can inspect that rather than relying on certain words being in the error string. I'm guessing the translated string would be most appropriate for end users. -- Joe Nelson https://begriffs.com diff --git a/contrib/pg_standby/Makefile b/contrib/pg_standby/Makefile index 0bca2f8e9e..cb9292d0f4 100644 --- a/contrib/pg_standby/Makefile +++ b/contrib/pg_standby/Makefile @@ -6,6 +6,8 @@ PGAPPICON = win32 PROGRAM = pg_standby OBJS = pg_standby.o $(WIN32RES) +PG_LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils + ifdef USE_PGXS PG_CONFIG = pg_config PGXS := $(shell $(PG_CONFIG) --pgxs) diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c index 031b1b5cd5..9ce736249b 100644 --- a/contrib/pg_standby/pg_standby.c +++ b/contrib/pg_standby/pg_standby.c @@ -33,6 +33,7 @@ #include "pg_getopt.h" #include "access/xlog_internal.h" +#include "fe_utils/option.h" const char *progname; @@ -678,6 +679,10 @@ main(int argc, char **argv) while ((c = getopt(argc, argv, "cdk:lr:s:t:w:")) != -1) { + pg_strtoint_status s; + int64 parsed; + char *parse_error; + switch (c) { case 'c': /* Use copy */ @@ -687,12 +692,15 @@ main(int argc, char **argv) debug = true; break; case 'k': /* keepfiles */ - keepfiles = atoi(optarg); - if (keepfiles < 0) + s = pg_strtoint64_range(optarg, &parsed, + 0, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - fprintf(stderr, "%s: -k keepfiles must be >= 0\n", progname); + fprintf(stderr, "%s: invalid argument for -k keepfiles: %s\n", + progname, parse_error); exit(2); } + keepfiles = parsed; break; case 'l': /* Use link */ @@ -706,31 +714,39 @@ main(int argc, char **argv) #endif break; case 'r': /* Retries */ - maxretries = atoi(optarg); - if (maxretries < 0) + s = pg_strtoint64_range(optarg, &parsed, + 0, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - fprintf(stderr, "%s: -r maxretries must be >= 0\n", progname); + fprintf(stderr, "%s: invalid argument for -r maxretries: %s\n", + progname, parse_error); exit(2); } + maxretries = parsed; break; case 's': /* Sleep time */ - sleeptime = atoi(optarg); - if (sleeptime <= 0 || sleeptime > 60) + s = pg_strtoint64_range(optarg, &parsed, 1, 60, &parse_error); + if (s != PG_STRTOINT_OK) { - fprintf(stderr, "%s: -s sleeptime incorrectly set\n", progname); + fprintf(stderr, "%s: invalid argument for -s sleeptime: %s\n", + progname, parse_error); exit(2); } + sleeptime = parsed; break; case 't': /* Trigger file */ triggerPath = pg_strdup(optarg); break; case 'w': /* Max wait time */ - maxwaittime = atoi(optarg); - if (maxwaittime < 0) + s = pg_strtoint64_range(optarg, &parsed, + 0, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - fprintf(stderr, "%s: -w maxwaittime incorrectly set\n", progname); + fprintf(stderr, "%s: invalid argument for -w maxwaittime: %s\n", + progname, parse_error); exit(2); } + maxwaittime = parsed; break; default: fprintf(stderr, "Try \"%s --help\" for more information.\n", progname); diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 55ef13926d..7869c8cf9a 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -32,6 +32,7 @@ #include "common/logging.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" +#include "fe_utils/option.h" #include "fe_utils/string_utils.h" #include "getopt_long.h" #include "libpq-fe.h" @@ -2073,6 +2074,10 @@ 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) { + pg_strtoint_status s; + int64 parsed; + char *parse_error; + switch (c) { case 'C': @@ -2157,12 +2162,13 @@ main(int argc, char **argv) #endif break; case 'Z': - compresslevel = atoi(optarg); - if (compresslevel < 0 || compresslevel > 9) + s = pg_strtoint64_range(optarg, &parsed, 0, 9, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("invalid compression level \"%s\"", optarg); + pg_log_error("invalid compression level: %s", parse_error); exit(1); } + compresslevel = parsed; break; case 'c': if (pg_strcasecmp(optarg, "fast") == 0) @@ -2195,12 +2201,14 @@ main(int argc, char **argv) dbgetpassword = 1; break; case 's': - standby_message_timeout = atoi(optarg) * 1000; - if (standby_message_timeout < 0) + s = pg_strtoint64_range(optarg, &parsed, + 0, INT_MAX / 1000, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("invalid status interval \"%s\"", optarg); + pg_log_error("invalid status interval: %s", parse_error); exit(1); } + standby_message_timeout = parsed * 1000; break; case 'v': verbose++; diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c index f39c1339d7..d6289f2f8f 100644 --- a/src/bin/pg_basebackup/pg_receivewal.c +++ b/src/bin/pg_basebackup/pg_receivewal.c @@ -21,6 +21,7 @@ #include "common/file_perm.h" #include "common/logging.h" +#include "fe_utils/option.h" #include "libpq-fe.h" #include "access/xlog_internal.h" #include "getopt_long.h" @@ -492,7 +493,6 @@ main(int argc, char **argv) {"no-sync", no_argument, NULL, 5}, {NULL, 0, NULL, 0} }; - int c; int option_index; char *db_name; @@ -521,6 +521,10 @@ 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) { + pg_strtoint_status s; + int64 parsed; + char *parse_error; + switch (c) { case 'D': @@ -533,11 +537,14 @@ main(int argc, char **argv) dbhost = pg_strdup(optarg); break; case 'p': - if (atoi(optarg) <= 0) + s = pg_strtoint64_range(optarg, &parsed, FE_UTILS_PORT_MIN, + FE_UTILS_PORT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("invalid port number \"%s\"", optarg); + pg_log_error("invalid port number: %s", parse_error); exit(1); } + /* validated conversion above, but using the string */ dbport = pg_strdup(optarg); break; case 'U': @@ -550,12 +557,14 @@ main(int argc, char **argv) dbgetpassword = 1; break; case 's': - standby_message_timeout = atoi(optarg) * 1000; - if (standby_message_timeout < 0) + s = pg_strtoint64_range(optarg, &parsed, + 0, INT_MAX / 1000, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("invalid status interval \"%s\"", optarg); + pg_log_error("invalid status interval: %s", parse_error); exit(1); } + standby_message_timeout = parsed * 1000; break; case 'S': replication_slot = pg_strdup(optarg); @@ -575,12 +584,13 @@ main(int argc, char **argv) verbose++; break; case 'Z': - compresslevel = atoi(optarg); - if (compresslevel < 0 || compresslevel > 9) + s = pg_strtoint64_range(optarg, &parsed, 0, 9, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("invalid compression level \"%s\"", optarg); + pg_log_error("invalid compression level: %s", parse_error); exit(1); } + compresslevel = parsed; break; /* action */ case 1: diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c index af29dd7651..0c1437c989 100644 --- a/src/bin/pg_basebackup/pg_recvlogical.c +++ b/src/bin/pg_basebackup/pg_recvlogical.c @@ -26,6 +26,7 @@ #include "common/file_perm.h" #include "common/fe_memutils.h" #include "common/logging.h" +#include "fe_utils/option.h" #include "getopt_long.h" #include "libpq-fe.h" #include "libpq/pqsignal.h" @@ -705,6 +706,10 @@ main(int argc, char **argv) while ((c = getopt_long(argc, argv, "E:f:F:nvd:h:p:U:wWI:o:P:s:S:", long_options, &option_index)) != -1) { + pg_strtoint_status s; + int64 parsed; + char *parse_error; + switch (c) { /* general options */ @@ -712,12 +717,14 @@ main(int argc, char **argv) outfile = pg_strdup(optarg); break; case 'F': - fsync_interval = atoi(optarg) * 1000; - if (fsync_interval < 0) + s = pg_strtoint64_range(optarg, &parsed, + 0, INT_MAX / 1000, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("invalid fsync interval \"%s\"", optarg); + pg_log_error("invalid fsync interval: %s", parse_error); exit(1); } + fsync_interval = parsed * 1000; break; case 'n': noloop = 1; @@ -733,11 +740,14 @@ main(int argc, char **argv) dbhost = pg_strdup(optarg); break; case 'p': - if (atoi(optarg) <= 0) + s = pg_strtoint64_range(optarg, &parsed, FE_UTILS_PORT_MIN, + FE_UTILS_PORT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("invalid port number \"%s\"", optarg); + pg_log_error("invalid port number: %s", parse_error); exit(1); } + /* validated conversion above, but using the string */ dbport = pg_strdup(optarg); break; case 'U': @@ -790,12 +800,14 @@ main(int argc, char **argv) plugin = pg_strdup(optarg); break; case 's': - standby_message_timeout = atoi(optarg) * 1000; - if (standby_message_timeout < 0) + s = pg_strtoint64_range(optarg, &parsed, + 0, INT_MAX / 1000, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("invalid status interval \"%s\"", optarg); + pg_log_error("invalid status interval: %s", parse_error); exit(1); } + standby_message_timeout = parsed * 1000; break; case 'S': replication_slot = pg_strdup(optarg); diff --git a/src/bin/pg_ctl/Makefile b/src/bin/pg_ctl/Makefile index 83cbf97ed8..f7d375f869 100644 --- a/src/bin/pg_ctl/Makefile +++ b/src/bin/pg_ctl/Makefile @@ -24,6 +24,7 @@ LDFLAGS_INTERNAL += $(libpq_pgport) SUBMAKE_LIBPQ := submake-libpq endif +LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils OBJS= pg_ctl.o $(WIN32RES) all: pg_ctl diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index dd76be6dd2..ad03a0d080 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -28,6 +28,7 @@ #include "common/file_perm.h" #include "common/logging.h" #include "common/string.h" +#include "fe_utils/option.h" #include "getopt_long.h" #include "utils/pidfile.h" @@ -2332,6 +2333,10 @@ main(int argc, char **argv) while ((c = getopt_long(argc, argv, "cD:e:l:m:N:o:p:P:sS:t:U:wW", long_options, &option_index)) != -1) { + pg_strtoint_status s; + int64 parsed; + char *parse_error; + switch (c) { case 'D': @@ -2395,7 +2400,14 @@ main(int argc, char **argv) #endif break; case 't': - wait_seconds = atoi(optarg); + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) + { + write_stderr(_("invalid timeout: %s\n"), parse_error); + exit(1); + } + wait_seconds = parsed; wait_seconds_arg = true; break; case 'U': diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index f01fea5b91..265e88fbab 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -62,6 +62,7 @@ #include "pg_backup_db.h" #include "pg_backup_utils.h" #include "pg_dump.h" +#include "fe_utils/option.h" #include "fe_utils/connect.h" #include "fe_utils/string_utils.h" @@ -430,6 +431,10 @@ main(int argc, char **argv) while ((c = getopt_long(argc, argv, "abBcCd:E:f:F:h:j:n:N:Op:RsS:t:T:U:vwWxZ:", long_options, &optindex)) != -1) { + pg_strtoint_status s; + int64 parsed; + char *parse_error; + switch (c) { case 'a': /* Dump data only */ @@ -473,7 +478,14 @@ main(int argc, char **argv) break; case 'j': /* number of dump jobs */ - numWorkers = atoi(optarg); + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) + { + pg_log_error("invalid job count: %s", parse_error); + exit_nicely(1); + } + numWorkers = parsed; break; case 'n': /* include schema(s) */ @@ -536,12 +548,13 @@ main(int argc, char **argv) break; case 'Z': /* Compression Level */ - compressLevel = atoi(optarg); - if (compressLevel < 0 || compressLevel > 9) + s = pg_strtoint64_range(optarg, &parsed, 0, 9, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("compression level must be in range 0..9"); + pg_log_error("invalid compression level: %s", parse_error); exit_nicely(1); } + compressLevel = parsed; break; case 0: @@ -574,12 +587,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) + s = pg_strtoint64_range(optarg, &parsed, -15, 3, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("extra_float_digits must be in range -15..3"); + pg_log_error("invalid extra_float_digits: %s", parse_error); exit_nicely(1); } + extra_float_digits = parsed; break; case 9: /* inserts */ diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c index 40a6b3745c..b01c169c14 100644 --- a/src/bin/pg_dump/pg_restore.c +++ b/src/bin/pg_dump/pg_restore.c @@ -45,6 +45,7 @@ #include <termios.h> #endif +#include "fe_utils/option.h" #include "getopt_long.h" #include "dumputils.h" @@ -153,6 +154,10 @@ 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) { + pg_strtoint_status s; + int64 parsed; + char *parse_error; + switch (c) { case 'a': /* Dump data only */ @@ -183,7 +188,14 @@ main(int argc, char **argv) break; case 'j': /* number of restore jobs */ - numWorkers = atoi(optarg); + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) + { + pg_log_error("invalid job count: %s", parse_error); + exit_nicely(1); + } + numWorkers = parsed; break; case 'l': /* Dump the TOC summary */ diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index 28ff4c48ed..8e8cffaed2 100644 --- a/src/bin/pg_upgrade/option.c +++ b/src/bin/pg_upgrade/option.c @@ -14,6 +14,7 @@ #include <io.h> #endif +#include "fe_utils/option.h" #include "getopt_long.h" #include "common/string.h" #include "utils/pidfile.h" @@ -106,6 +107,10 @@ 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) { + pg_strtoint_status s; + int64 parsed; + char *parse_error; + switch (option) { case 'b': @@ -129,7 +134,14 @@ parseCommandLine(int argc, char *argv[]) break; case 'j': - user_opts.jobs = atoi(optarg); + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) + { + pg_fatal("invalid job count: %s\n", parse_error); + exit(1); + } + user_opts.jobs = parsed; break; case 'k': @@ -168,19 +180,25 @@ 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) + s = pg_strtoint64_range(optarg, &parsed, FE_UTILS_PORT_MIN, + FE_UTILS_PORT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_fatal("invalid old port number\n"); + pg_fatal("invalid old port number: %s\n", parse_error); exit(1); } + old_cluster.port = parsed; break; case 'P': - if ((new_cluster.port = atoi(optarg)) <= 0) + s = pg_strtoint64_range(optarg, &parsed, FE_UTILS_PORT_MIN, + FE_UTILS_PORT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_fatal("invalid new port number\n"); + pg_fatal("invalid new port number: %s\n", parse_error); exit(1); } + new_cluster.port = parsed; break; case 'r': diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 600f1deb71..7eb3a6ff63 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -36,6 +36,7 @@ #include "common/logging.h" #include "common/string.h" #include "fe_utils/conditional.h" +#include "fe_utils/option.h" #include "getopt_long.h" #include "libpq-fe.h" #include "portability/instr_time.h" @@ -5094,6 +5095,9 @@ 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; + pg_strtoint_status s; + int64 parsed; + char *parse_error; switch (c) { @@ -5125,13 +5129,15 @@ main(int argc, char **argv) break; case 'c': benchmarking_option_set = true; - nclients = atoi(optarg); - if (nclients <= 0) + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - fprintf(stderr, "invalid number of clients: \"%s\"\n", - optarg); + fprintf(stderr, "invalid number of clients: %s\n", + parse_error); exit(1); } + nclients = parsed; #ifdef HAVE_GETRLIMIT #ifdef RLIMIT_NOFILE /* most platforms use RLIMIT_NOFILE */ if (getrlimit(RLIMIT_NOFILE, &rlim) == -1) @@ -5153,13 +5159,15 @@ main(int argc, char **argv) break; case 'j': /* jobs */ benchmarking_option_set = true; - nthreads = atoi(optarg); - if (nthreads <= 0) + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - fprintf(stderr, "invalid number of threads: \"%s\"\n", - optarg); + fprintf(stderr, "invalid number of threads: %s\n", + parse_error); exit(1); } + nthreads = parsed; #ifndef ENABLE_THREAD_SAFETY if (nthreads != 1) { @@ -5178,31 +5186,37 @@ main(int argc, char **argv) break; case 's': scale_given = true; - scale = atoi(optarg); - if (scale <= 0) + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - fprintf(stderr, "invalid scaling factor: \"%s\"\n", optarg); + fprintf(stderr, "invalid scaling factor: %s\n", parse_error); exit(1); } + scale = parsed; break; case 't': benchmarking_option_set = true; - nxacts = atoi(optarg); - if (nxacts <= 0) + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - fprintf(stderr, "invalid number of transactions: \"%s\"\n", - optarg); + fprintf(stderr, "invalid number of transactions: %s\n", + parse_error); exit(1); } + nxacts = parsed; break; case 'T': benchmarking_option_set = true; - duration = atoi(optarg); - if (duration <= 0) + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { fprintf(stderr, "invalid duration: \"%s\"\n", optarg); exit(1); } + duration = parsed; break; case 'U': login = pg_strdup(optarg); @@ -5261,12 +5275,14 @@ main(int argc, char **argv) break; case 'F': initialization_option_set = true; - fillfactor = atoi(optarg); - if (fillfactor < 10 || fillfactor > 100) + s = pg_strtoint64_range(optarg, &parsed, + 10, 100, &parse_error); + if (s != PG_STRTOINT_OK) { - fprintf(stderr, "invalid fillfactor: \"%s\"\n", optarg); + fprintf(stderr, "invalid fillfactor: %s\n", parse_error); exit(1); } + fillfactor = parsed; break; case 'M': benchmarking_option_set = true; @@ -5282,13 +5298,15 @@ main(int argc, char **argv) break; case 'P': benchmarking_option_set = true; - progress = atoi(optarg); - if (progress <= 0) + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - fprintf(stderr, "invalid thread progress delay: \"%s\"\n", - optarg); + fprintf(stderr, "invalid thread progress delay: %s\n", + parse_error); exit(1); } + progress = parsed; break; case 'R': { @@ -5343,13 +5361,15 @@ main(int argc, char **argv) break; case 5: /* aggregate-interval */ benchmarking_option_set = true; - agg_interval = atoi(optarg); - if (agg_interval <= 0) + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - fprintf(stderr, "invalid number of seconds for aggregation: \"%s\"\n", - optarg); + fprintf(stderr, "invalid number of seconds for aggregation: %s\n", + parse_error); exit(1); } + agg_interval = parsed; break; case 6: /* progress-timestamp */ progress_timestamp = true; diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c index f00aec15de..5024aaad67 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -15,6 +15,7 @@ #include "common.h" #include "common/logging.h" #include "fe_utils/connect.h" +#include "fe_utils/option.h" #include "fe_utils/simple_list.h" #include "fe_utils/string_utils.h" #include "scripts_parallel.h" @@ -105,6 +106,10 @@ 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) { + pg_strtoint_status s; + int64 parsed; + char *parse_error; + switch (c) { case 'h': @@ -147,12 +152,14 @@ main(int argc, char *argv[]) simple_string_list_append(&indexes, optarg); break; case 'j': - concurrentCons = atoi(optarg); - if (concurrentCons <= 0) + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("number of parallel jobs must be at least 1"); + pg_log_error("invalid number of parallel jobs: %s", parse_error); exit(1); } + concurrentCons = parsed; break; case 'v': verbose = true; diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c index 2c7219239f..9266966d62 100644 --- a/src/bin/scripts/vacuumdb.c +++ b/src/bin/scripts/vacuumdb.c @@ -17,6 +17,7 @@ #include "common.h" #include "common/logging.h" #include "fe_utils/connect.h" +#include "fe_utils/option.h" #include "fe_utils/simple_list.h" #include "fe_utils/string_utils.h" #include "scripts_parallel.h" @@ -124,6 +125,10 @@ main(int argc, char *argv[]) while ((c = getopt_long(argc, argv, "h:p:U:wWeqd:zZFat:fvj:", long_options, &optindex)) != -1) { + pg_strtoint_status s; + int64 parsed; + char *parse_error; + switch (c) { case 'h': @@ -175,12 +180,14 @@ main(int argc, char *argv[]) vacopts.verbose = true; break; case 'j': - concurrentCons = atoi(optarg); - if (concurrentCons <= 0) + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("number of parallel jobs must be at least 1"); + pg_log_error("invalid number of parallel jobs: %s", parse_error); exit(1); } + concurrentCons = parsed; break; case 2: maintenance_db = pg_strdup(optarg); @@ -195,20 +202,24 @@ main(int argc, char *argv[]) vacopts.skip_locked = true; break; case 6: - vacopts.min_xid_age = atoi(optarg); - if (vacopts.min_xid_age <= 0) + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("minimum transaction ID age must be at least 1"); + pg_log_error("invalid minimum transaction ID age: %s", parse_error); exit(1); } + vacopts.min_xid_age = parsed; break; case 7: - vacopts.min_mxid_age = atoi(optarg); - if (vacopts.min_mxid_age <= 0) + s = pg_strtoint64_range(optarg, &parsed, + 1, INT_MAX, &parse_error); + if (s != PG_STRTOINT_OK) { - pg_log_error("minimum multixact ID age must be at least 1"); + pg_log_error("invalid minimum multixact ID age: %s", parse_error); exit(1); } + vacopts.min_mxid_age = parsed; break; default: fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname); diff --git a/src/fe_utils/Makefile b/src/fe_utils/Makefile index f2e516a2aa..83063abdcd 100644 --- a/src/fe_utils/Makefile +++ b/src/fe_utils/Makefile @@ -19,8 +19,8 @@ include $(top_builddir)/src/Makefile.global override CPPFLAGS := -DFRONTEND -I$(libpq_srcdir) $(CPPFLAGS) -OBJS = conditional.o mbprint.o print.o psqlscan.o recovery_gen.o \ - simple_list.o string_utils.o +OBJS = conditional.o mbprint.o option.o print.o psqlscan.o \ + recovery_gen.o simple_list.o string_utils.o all: libpgfeutils.a diff --git a/src/fe_utils/option.c b/src/fe_utils/option.c new file mode 100644 index 0000000000..3924166718 --- /dev/null +++ b/src/fe_utils/option.c @@ -0,0 +1,46 @@ +/*------------------------------------------------------------------------- + * + * option.c + * argument parsing helpers for frontend code + * + * Copyright (c) 2019, PostgreSQL Global Development Group + * + * + * IDENTIFICATION + * src/fe_utils/option.c + * + *------------------------------------------------------------------------- + */ + +#include "postgres_fe.h" + +#include "fe_utils/option.h" + +pg_strtoint_status +pg_strtoint64_range(const char *str, int64 *result, + int64 min, int64 max, char **error) +{ + int64 temp; + pg_strtoint_status s = pg_strtoint64(str, &temp); + + if (s == PG_STRTOINT_OK && (temp < min || temp > max)) + s = PG_STRTOINT_RANGE_ERROR; + + switch (s) + { + case PG_STRTOINT_OK: + *result = temp; + break; + case PG_STRTOINT_SYNTAX_ERROR: + *error = psprintf(_("could not parse '%s' as integer"), str); + break; + case PG_STRTOINT_RANGE_ERROR: + *error = psprintf(_("%s is outside range " + INT64_FORMAT ".." INT64_FORMAT), + str, min, max); + break; + default: + pg_unreachable(); + } + return s; +} diff --git a/src/include/fe_utils/option.h b/src/include/fe_utils/option.h new file mode 100644 index 0000000000..5833161fe5 --- /dev/null +++ b/src/include/fe_utils/option.h @@ -0,0 +1,31 @@ +/* + * option.h + * argument parsing helpers for frontend code + * + * Copyright (c) 2019, PostgreSQL Global Development Group + * + * src/include/fe_utils/option.h + */ +#ifndef FE_OPTION_H +#define FE_OPTION_H + +#include <limits.h> + +#include "common/string.h" + +#define FE_UTILS_PORT_MIN 1024 +#define FE_UTILS_PORT_MAX ((1 << 16) - 1) + +/* + * Parses string as int64 like pg_strtoint64, but fails + * with PG_STRTOINT_RANGE_ERROR if the result is outside + * the range min .. max inclusive. + * + * On failure, creates user-friendly error message with + * psprintf, and assigns it to the error output parameter. + */ +pg_strtoint_status +pg_strtoint64_range(const char *str, int64 *result, + int64 min, int64 max, char **error); + +#endif /* FE_OPTION_H */ diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index 7a103e6140..a9a01d22b7 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -142,7 +142,7 @@ sub mkvcbuild our @pgcommonbkndfiles = @pgcommonallfiles; our @pgfeutilsfiles = qw( - conditional.c mbprint.c print.c psqlscan.l psqlscan.c + conditional.c mbprint.c option.c print.c psqlscan.l psqlscan.c simple_list.c string_utils.c recovery_gen.c); $libpgport = $solution->AddProject('libpgport', 'lib', 'misc');
pgsql-hackers by date: