Hi Yesterday while doing a few pg_basebackup, I realized that the integer parameters were not properly checked against invalid input. It is not a critical issue, but this could be misleading for an user who writes -z max or -s 0.5… I've attached the patch to this mail. Should I add it to the next commit fest or is it not needed for such small patches ?
Thanks Pierre
From c5301085514b8e5acd9ffa5b10ae6521677b4d72 Mon Sep 17 00:00:00 2001
From: Pierre Ducroquet <p.p...@pinaraf.info>
Date: Thu, 13 Apr 2017 23:25:51 +0200
Subject: [PATCH] Check the results of atoi in pg_basebackup
Passing invalid strings in integer parameters does not trigger any error
in pg_basebackup right now. This behaviour could be misleading and give
the impression that a setting is considered (-z max, -s 0.5) while it
was reset to 0 instead.
---
src/bin/pg_basebackup/pg_basebackup.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 40ec0e17dc..e69dbd1ed6 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;
@@ -2063,6 +2063,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'},
@@ -2203,8 +2204,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);
@@ -2242,8 +2243,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);
--
2.11.0
signature.asc
Description: This is a digitally signed message part.
