Re: Change atoi to strtol in same place

Поиск
Список
Период
Сортировка
От Joe Nelson
Тема Re: Change atoi to strtol in same place
Дата
Msg-id 20191007002150.GD68117@begriffs.com
обсуждение исходный текст
Ответ на Re: Change atoi to strtol in same place  (Ashutosh Sharma <ashu.coek88@gmail.com>)
Ответы Re: Change atoi to strtol in same place  (David Rowley <david.rowley@2ndquadrant.com>)
Список 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 по дате отправления:

Предыдущее
От: Andrew Gierth
Дата:
Сообщение: Re: v12 and pg_restore -f-
Следующее
От: Amit Langote
Дата:
Сообщение: adding partitioned tables to publications