On Wednesday, September 13, 2017 2:06:50 AM CEST Daniel Gustafsson wrote:
> > On 05 Jul 2017, at 08:32, Michael Paquier <michael.paqu...@gmail.com>
> > wrote:>
> > On Wed, Jul 5, 2017 at 2:57 PM, Ryan Murphy <ryanfmur...@gmail.com> wrote:
> >> I tried to apply your patch to test it (though reading Robert's last
> >> comment it seems we wish to have it adjusted before committing)... but
> >> in any case I was not able to apply your patch to the tip of the master
> >> branch (my git apply failed). I'm setting this to Waiting On Author for
> >> a new patch, and I also agree with Robert that the test can be simpler
> >> and can go in the other order. If you don't have time to make another
> >> patch, let me know, I may be able to make one.>
> > git is unhappy even if forcibly applied with patch -p1. You should
> > check for whitespaces at the same time:
> > $ git diff --check
> > src/bin/pg_basebackup/pg_receivewal.c:483: indent with spaces.
> > + char *strtol_endptr = NULL
> > And there are more than this one.
>
> Like Michael said above, this patch no longer applies and have some
> whitespace issues. The conflicts in applying are quite trivial though, so
> it should be easy to create a new version. Do you plan to work on this
> during this Commitfest so we can move this patch forward?
Yes, my bad. Attached to this email is the new version, that now applies on
master.
Sorry for the delay :(
>From 7efc1573c1bcc07c0eaa80912e6e035f2e0d203d Mon Sep 17 00:00:00 2001
From: Pierre Ducroquet <p.p...@pinaraf.info>
Date: Wed, 13 Sep 2017 19:51:09 +0200
Subject: [PATCH] Port most calls of atoi(optarg) to strcol
atoi does not allow any kind of data check. If the input data is invalid,
the result will be silently truncated to the valid part of input, or just
0 if no digit was found at the beginning of input.
---
src/bin/pg_basebackup/pg_basebackup.c | 11 ++++++-----
src/bin/pg_basebackup/pg_receivewal.c | 11 ++++++-----
src/bin/pg_basebackup/pg_recvlogical.c | 11 ++++++-----
src/bin/pg_ctl/pg_ctl.c | 9 ++++++++-
src/bin/pg_dump/pg_dump.c | 12 +++++++++---
src/bin/pg_dump/pg_restore.c | 8 +++++++-
src/bin/pg_upgrade/option.c | 5 +++--
src/bin/pgbench/pgbench.c | 33 +++++++++++++++++----------------
8 files changed, 62 insertions(+), 38 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 51509d150e..b51b62cc21 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -87,7 +87,7 @@ static IncludeWal includewal = STREAM_WAL;
static bool fastcheckpoint = false;
static bool writerecoveryconf = false;
static bool do_sync = true;
-static int standby_message_timeout = 10 * 1000; /* 10 sec = default */
+static long int standby_message_timeout = 10 * 1000; /* 10 sec = default */
static pg_time_t last_progress_report = 0;
static int32 maxrate = 0; /* no limit by default */
static char *replication_slot = NULL;
@@ -2072,6 +2072,7 @@ BaseBackup(void)
int
main(int argc, char **argv)
{
+ char *strtol_endptr = NULL;
static struct option long_options[] = {
{"help", no_argument, NULL, '?'},
{"version", no_argument, NULL, 'V'},
@@ -2212,8 +2213,8 @@ main(int argc, char **argv)
#endif
break;
case 'Z':
- compresslevel = atoi(optarg);
- if (compresslevel < 0 || compresslevel > 9)
+ compresslevel = strtol(optarg, &strtol_endptr, 10);
+ if (compresslevel < 0 || compresslevel > 9 || (strtol_endptr != optarg + strlen(optarg)))
{
fprintf(stderr, _("%s: invalid compression level \"%s\"\n"),
progname, optarg);
@@ -2251,8 +2252,8 @@ main(int argc, char **argv)
dbgetpassword = 1;
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
+ standby_message_timeout = strtol(optarg, &strtol_endptr, 10) * 1000;
+ if ((standby_message_timeout < 0) || (strtol_endptr != optarg + strlen(optarg)))
{
fprintf(stderr, _("%s: invalid status interval \"%s\"\n"),
progname, optarg);
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index 710a33ab4d..c1651961b5 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -494,6 +494,7 @@ main(int argc, char **argv)
int c;
int option_index;
char *db_name;
+ char *strtol_endptr = NULL;
uint32 hi, lo;
progname = get_progname(argv[0]);
@@ -529,7 +530,7 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
+ if ((strtol(optarg, &strtol_endptr, 10) <= 0) || (strtol_endptr != optarg + strlen(optarg)))
{
fprintf(stderr, _("%s: invalid port number \"%s\"\n"),
progname, optarg);
@@ -547,8 +548,8 @@ main(int argc, char **argv)
dbgetpassword = 1;
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
+ standby_message_timeout = strtol(optarg, &strtol_endptr, 10) * 1000;
+ if ((standby_message_timeout < 0) || (strtol_endptr != optarg + strlen(optarg)))
{
fprintf(stderr, _("%s: invalid status interval \"%s\"\n"),
progname, optarg);
@@ -575,8 +576,8 @@ main(int argc, char **argv)
verbose++;
break;
case 'Z':
- compresslevel = atoi(optarg);
- if (compresslevel < 0 || compresslevel > 9)
+ compresslevel = strtol(optarg, &strtol_endptr, 10);
+ if (compresslevel < 0 || compresslevel > 9 || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, _("%s: invalid compression level \"%s\"\n"),
progname, optarg);
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 6811a55e76..bde45b2650 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -706,6 +706,7 @@ main(int argc, char **argv)
uint32 hi,
lo;
char *db_name;
+ char *strtol_endptr = NULL;
progname = get_progname(argv[0]);
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_basebackup"));
@@ -735,8 +736,8 @@ main(int argc, char **argv)
outfile = pg_strdup(optarg);
break;
case 'F':
- fsync_interval = atoi(optarg) * 1000;
- if (fsync_interval < 0)
+ fsync_interval = strtol(optarg, &strtol_endptr, 10) * 1000;
+ if (fsync_interval < 0 || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, _("%s: invalid fsync interval \"%s\"\n"),
progname, optarg);
@@ -757,7 +758,7 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
+ if (strtol(optarg, &strtol_endptr, 10) <= 0 || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, _("%s: invalid port number \"%s\"\n"),
progname, optarg);
@@ -819,8 +820,8 @@ main(int argc, char **argv)
plugin = pg_strdup(optarg);
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
+ standby_message_timeout = strtol(optarg, &strtol_endptr, 10) * 1000;
+ if (standby_message_timeout < 0 || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, _("%s: invalid status interval \"%s\"\n"),
progname, optarg);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 4e02c4cea1..2190412293 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2100,6 +2100,7 @@ main(int argc, char **argv)
};
char *env_wait;
+ char *strtol_endptr = NULL;
int option_index;
int c;
pgpid_t killproc = 0;
@@ -2231,7 +2232,13 @@ main(int argc, char **argv)
#endif
break;
case 't':
- wait_seconds = atoi(optarg);
+ wait_seconds = strtol(optarg, &strtol_endptr, 10);
+ if (strtol_endptr != optarg + strlen(optarg))
+ {
+ write_stderr(_("%s: invalid value '%s' for -t\n"),
+ progname, optarg);
+ exit(1);
+ }
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 75f08cd792..ddbf742878 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -290,6 +290,7 @@ main(int argc, char **argv)
const char *dumpencoding = NULL;
const char *dumpsnapshot = NULL;
char *use_role = NULL;
+ char *strtol_endptr = NULL;
int numWorkers = 1;
trivalue prompt_password = TRI_DEFAULT;
int compressLevel = -1;
@@ -441,7 +442,12 @@ main(int argc, char **argv)
break;
case 'j': /* number of dump jobs */
- numWorkers = atoi(optarg);
+ numWorkers = strtol(optarg, &strtol_endptr, 10);
+ if (strtol_endptr != optarg + strlen(optarg))
+ {
+ write_msg(NULL, "invalid number of jobs\n");
+ exit_nicely(1);
+ }
break;
case 'n': /* include schema(s) */
@@ -507,8 +513,8 @@ main(int argc, char **argv)
break;
case 'Z': /* Compression Level */
- compressLevel = atoi(optarg);
- if (compressLevel < 0 || compressLevel > 9)
+ compressLevel = strtol(optarg, &strtol_endptr, 10);
+ if (compressLevel < 0 || compressLevel > 9 || strtol_endptr != optarg + strlen(optarg))
{
write_msg(NULL, "compression level must be in range 0..9\n");
exit_nicely(1);
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 860a211a3c..e759a6e960 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -65,6 +65,7 @@ main(int argc, char **argv)
int numWorkers = 1;
Archive *AH;
char *inputFileSpec;
+ char *strtol_endptr = NULL;
static int disable_triggers = 0;
static int enable_row_security = 0;
static int if_exists = 0;
@@ -181,7 +182,12 @@ main(int argc, char **argv)
break;
case 'j': /* number of restore jobs */
- numWorkers = atoi(optarg);
+ numWorkers = strtol(optarg, &strtol_endptr, 10);
+ if (strtol_endptr != optarg + strlen(optarg))
+ {
+ write_msg(NULL, "invalid number of jobs\n");
+ exit_nicely(1);
+ }
break;
case 'l': /* Dump the TOC summary */
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index c74eb25e18..678b5a8bfa 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -60,6 +60,7 @@ parseCommandLine(int argc, char *argv[])
int os_user_effective_id;
FILE *fp;
char **filename;
+ char *strtol_endptr = NULL;
time_t run_time = time(NULL);
user_opts.transfer_mode = TRANSFER_MODE_COPY;
@@ -167,7 +168,7 @@ 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)
+ if ((old_cluster.port = strtol(optarg, &strtol_endptr, 10)) <= 0 || strtol_endptr != optarg + strlen(optarg))
{
pg_fatal("invalid old port number\n");
exit(1);
@@ -175,7 +176,7 @@ parseCommandLine(int argc, char *argv[])
break;
case 'P':
- if ((new_cluster.port = atoi(optarg)) <= 0)
+ if ((new_cluster.port = strtol(optarg, &strtol_endptr, 10)) <= 0 || strtol_endptr != optarg + strlen(optarg))
{
pg_fatal("invalid new port number\n");
exit(1);
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index e37496c971..27746bcd89 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3708,6 +3708,7 @@ main(int argc, char **argv)
PGconn *con;
PGresult *res;
char *env;
+ char *strtol_endptr;
progname = get_progname(argv[0]);
@@ -3766,8 +3767,8 @@ main(int argc, char **argv)
break;
case 'c':
benchmarking_option_set = true;
- nclients = atoi(optarg);
- if (nclients <= 0 || nclients > MAXCLIENTS)
+ nclients = strtol(optarg, &strtol_endptr, 10);
+ if (nclients <= 0 || nclients > MAXCLIENTS || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, "invalid number of clients: \"%s\"\n",
optarg);
@@ -3794,8 +3795,8 @@ main(int argc, char **argv)
break;
case 'j': /* jobs */
benchmarking_option_set = true;
- nthreads = atoi(optarg);
- if (nthreads <= 0)
+ nthreads = strtol(optarg, &strtol_endptr, 10);
+ if (nthreads <= 0 || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, "invalid number of threads: \"%s\"\n",
optarg);
@@ -3820,8 +3821,8 @@ main(int argc, char **argv)
break;
case 's':
scale_given = true;
- scale = atoi(optarg);
- if (scale <= 0)
+ scale = strtol(optarg, &strtol_endptr, 10);
+ if (scale <= 0 || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, "invalid scaling factor: \"%s\"\n", optarg);
exit(1);
@@ -3834,8 +3835,8 @@ main(int argc, char **argv)
fprintf(stderr, "specify either a number of transactions (-t) or a duration (-T), not both\n");
exit(1);
}
- nxacts = atoi(optarg);
- if (nxacts <= 0)
+ nxacts = strtol(optarg, &strtol_endptr, 10);
+ if (nxacts <= 0 || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, "invalid number of transactions: \"%s\"\n",
optarg);
@@ -3849,8 +3850,8 @@ main(int argc, char **argv)
fprintf(stderr, "specify either a number of transactions (-t) or a duration (-T), not both\n");
exit(1);
}
- duration = atoi(optarg);
- if (duration <= 0)
+ duration = strtol(optarg, &strtol_endptr, 10);
+ if (duration <= 0 || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, "invalid duration: \"%s\"\n", optarg);
exit(1);
@@ -3916,8 +3917,8 @@ main(int argc, char **argv)
break;
case 'F':
initialization_option_set = true;
- fillfactor = atoi(optarg);
- if (fillfactor < 10 || fillfactor > 100)
+ fillfactor = strtol(optarg, &strtol_endptr, 10);
+ if (fillfactor < 10 || fillfactor > 100 || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, "invalid fillfactor: \"%s\"\n", optarg);
exit(1);
@@ -3937,8 +3938,8 @@ main(int argc, char **argv)
break;
case 'P':
benchmarking_option_set = true;
- progress = atoi(optarg);
- if (progress <= 0)
+ progress = strtol(optarg, &strtol_endptr, 10);
+ if (progress <= 0 || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, "invalid thread progress delay: \"%s\"\n",
optarg);
@@ -3999,8 +4000,8 @@ main(int argc, char **argv)
break;
case 5:
benchmarking_option_set = true;
- agg_interval = atoi(optarg);
- if (agg_interval <= 0)
+ agg_interval = strtol(optarg, &strtol_endptr, 10);
+ if (agg_interval <= 0 || strtol_endptr != optarg + strlen(optarg))
{
fprintf(stderr, "invalid number of seconds for aggregation: \"%s\"\n",
optarg);
--
2.14.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
