On Sat, Jul 24, 2021 at 07:41:12PM +0900, Michael Paquier wrote: > I have looked at that over the last couple of days, and applied it > after some small fixes, including an indentation.
One thing that we forgot here is the handling of trailing whitespaces. Attached is small patch to adjust that, with one positive and one negative tests. > The int64 and float > parts are extra types we could handle, but I have not looked yet at > how much benefits we'd get in those cases. I have looked at these two but there is really less benefits, so for now I am not planning to do more in this area. For float options, pg_basebackup --max-rate could be one target on top of the three set of options in pgbench, but it needs to handle units :( -- Michael
From 2c38b36841965fc458dab69d846bcc0dde07aca2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 26 Jul 2021 14:41:15 +0900
Subject: [PATCH] Skip trailing whitespaces when parsing integer options
strtoint(), via strtol(), would skip any leading whitespace but the same
rule was not applied for trailing whitespaces, which was a bit
inconsistent. Some tests are added while on it.
---
src/fe_utils/option_utils.c | 9 ++++++++-
src/bin/pg_basebackup/t/020_pg_receivewal.pl | 4 +++-
src/bin/pg_dump/t/001_basic.pl | 3 ++-
3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/fe_utils/option_utils.c b/src/fe_utils/option_utils.c
index 3e7e512ad9..bcfe7365fd 100644
--- a/src/fe_utils/option_utils.c
+++ b/src/fe_utils/option_utils.c
@@ -57,7 +57,14 @@ option_parse_int(const char *optarg, const char *optname,
errno = 0;
val = strtoint(optarg, &endptr, 10);
- if (*endptr)
+ /*
+ * Skip any trailing whitespace; if anything but whitespace remains before
+ * the terminating character, fail.
+ */
+ while (*endptr != '\0' && isspace((unsigned char) *endptr))
+ endptr++;
+
+ if (*endptr != '\0')
{
pg_log_error("invalid value \"%s\" for option %s",
optarg, optname);
diff --git a/src/bin/pg_basebackup/t/020_pg_receivewal.pl b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
index 158f7d176f..47c4ecb073 100644
--- a/src/bin/pg_basebackup/t/020_pg_receivewal.pl
+++ b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
@@ -88,10 +88,12 @@ SKIP:
$primary->psql('postgres',
'INSERT INTO test_table VALUES (generate_series(100,200));');
+ # Note the trailing whitespace after the value of --compress, that is
+ # a valid value.
$primary->command_ok(
[
'pg_receivewal', '-D', $stream_dir, '--verbose',
- '--endpos', $nextlsn, '--compress', '1'
+ '--endpos', $nextlsn, '--compress', '1 '
],
"streaming some WAL using ZLIB compression");
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 59de6df7ba..d1a7e1db40 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -101,8 +101,9 @@ command_fails_like(
qr/\Qpg_dump: error: parallel backup only supported by the directory format\E/,
'pg_dump: parallel backup only supported by the directory format');
+# Note the trailing whitespace for the value of --jobs, that is valid.
command_fails_like(
- [ 'pg_dump', '-j', '-1' ],
+ [ 'pg_dump', '-j', '-1 ' ],
qr/\Qpg_dump: error: -j\/--jobs must be in range\E/,
'pg_dump: -j/--jobs must be in range');
--
2.32.0
signature.asc
Description: PGP signature
