agora inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH 1/2] Be strict in numeric parameters on command line
27+ messages / 9 participants
[nested] [flat]

* [PATCH 1/2] Be strict in numeric parameters on command line
@ 2021-07-08 06:08 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Kyotaro Horiguchi @ 2021-07-08 06:08 UTC (permalink / raw)

Some numeric command line parameters are tolerant of valid values
followed by garbage like "123xyz".  Be strict to reject such invalid
values. Do the same for psql meta command parameters.
---
 src/bin/pg_amcheck/pg_amcheck.c        |  6 ++-
 src/bin/pg_basebackup/pg_basebackup.c  | 13 +++--
 src/bin/pg_basebackup/pg_receivewal.c  | 18 +++++--
 src/bin/pg_basebackup/pg_recvlogical.c | 17 +++++--
 src/bin/pg_checksums/pg_checksums.c    |  7 ++-
 src/bin/pg_ctl/pg_ctl.c                | 18 ++++++-
 src/bin/pg_dump/pg_dump.c              | 39 ++++++++-------
 src/bin/pg_dump/pg_restore.c           | 17 ++++---
 src/bin/pg_upgrade/option.c            | 21 ++++++--
 src/bin/pgbench/pgbench.c              | 66 ++++++++++++++++----------
 src/bin/psql/command.c                 | 52 ++++++++++++++++++--
 src/bin/scripts/reindexdb.c            | 10 ++--
 src/bin/scripts/vacuumdb.c             | 23 +++++----
 13 files changed, 219 insertions(+), 88 deletions(-)

diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 4bde16fb4b..71a82f9b75 100644
--- a/src/bin/pg_amcheck/pg_amcheck.c
+++ b/src/bin/pg_amcheck/pg_amcheck.c
@@ -17,6 +17,7 @@
 #include "catalog/pg_am_d.h"
 #include "catalog/pg_namespace_d.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "common/username.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/option_utils.h"
@@ -326,8 +327,9 @@ main(int argc, char *argv[])
 				append_btree_pattern(&opts.exclude, optarg, encoding);
 				break;
 			case 'j':
-				opts.jobs = atoi(optarg);
-				if (opts.jobs < 1)
+				errno = 0;
+				opts.jobs = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || opts.jobs < 1)
 				{
 					pg_log_error("number of parallel jobs must be at least 1");
 					exit(1);
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8bb0acf498..29be95b96a 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2287,6 +2287,8 @@ 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)
 	{
+		char   *endptr;
+
 		switch (c)
 		{
 			case 'C':
@@ -2371,8 +2373,10 @@ main(int argc, char **argv)
 #endif
 				break;
 			case 'Z':
-				compresslevel = atoi(optarg);
-				if (compresslevel < 0 || compresslevel > 9)
+				errno = 0;
+				compresslevel = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					compresslevel < 0 || compresslevel > 9)
 				{
 					pg_log_error("invalid compression level \"%s\"", optarg);
 					exit(1);
@@ -2409,8 +2413,9 @@ main(int argc, char **argv)
 				dbgetpassword = 1;
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				errno = 0;
+				standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr || errno == ERANGE || standby_message_timeout < 0)
 				{
 					pg_log_error("invalid status interval \"%s\"", optarg);
 					exit(1);
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index c1334fad35..7fef925b99 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -22,6 +22,7 @@
 #include "access/xlog_internal.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "getopt_long.h"
 #include "libpq-fe.h"
 #include "receivelog.h"
@@ -520,6 +521,8 @@ 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)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'D':
@@ -532,7 +535,9 @@ main(int argc, char **argv)
 				dbhost = pg_strdup(optarg);
 				break;
 			case 'p':
-				if (atoi(optarg) <= 0)
+				errno = 0;
+				if (strtoint(optarg, &endptr, 10) <= 0 ||
+					*endptr || errno == ERANGE)
 				{
 					pg_log_error("invalid port number \"%s\"", optarg);
 					exit(1);
@@ -549,8 +554,9 @@ main(int argc, char **argv)
 				dbgetpassword = 1;
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				errno = 0;
+				standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr || errno == ERANGE || standby_message_timeout < 0)
 				{
 					pg_log_error("invalid status interval \"%s\"", optarg);
 					exit(1);
@@ -574,8 +580,10 @@ main(int argc, char **argv)
 				verbose++;
 				break;
 			case 'Z':
-				compresslevel = atoi(optarg);
-				if (compresslevel < 0 || compresslevel > 9)
+				errno = 0;
+				compresslevel = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					compresslevel < 0 || compresslevel > 9)
 				{
 					pg_log_error("invalid compression level \"%s\"", optarg);
 					exit(1);
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 76bd153fac..7be932d025 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -23,6 +23,7 @@
 #include "common/fe_memutils.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "getopt_long.h"
 #include "libpq-fe.h"
 #include "libpq/pqsignal.h"
@@ -732,6 +733,8 @@ main(int argc, char **argv)
 	while ((c = getopt_long(argc, argv, "E:f:F:nvtd:h:p:U:wWI:o:P:s:S:",
 							long_options, &option_index)) != -1)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 /* general options */
@@ -739,8 +742,9 @@ main(int argc, char **argv)
 				outfile = pg_strdup(optarg);
 				break;
 			case 'F':
-				fsync_interval = atoi(optarg) * 1000;
-				if (fsync_interval < 0)
+				errno = 0;
+				fsync_interval = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr || errno == ERANGE || fsync_interval < 0)
 				{
 					pg_log_error("invalid fsync interval \"%s\"", optarg);
 					exit(1);
@@ -763,7 +767,9 @@ main(int argc, char **argv)
 				dbhost = pg_strdup(optarg);
 				break;
 			case 'p':
-				if (atoi(optarg) <= 0)
+				errno = 0;
+				if (strtoint(optarg, &endptr, 10) <= 0 ||
+					*endptr || errno == ERANGE)
 				{
 					pg_log_error("invalid port number \"%s\"", optarg);
 					exit(1);
@@ -820,8 +826,9 @@ main(int argc, char **argv)
 				plugin = pg_strdup(optarg);
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				errno = 0;
+				standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr || errno == ERANGE || standby_message_timeout < 0)
 				{
 					pg_log_error("invalid status interval \"%s\"", optarg);
 					exit(1);
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 3c326906e2..1c4e5b9d85 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -24,6 +24,7 @@
 #include "common/file_perm.h"
 #include "common/file_utils.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "getopt_long.h"
 #include "pg_getopt.h"
 #include "storage/bufpage.h"
@@ -506,6 +507,8 @@ main(int argc, char *argv[])
 
 	while ((c = getopt_long(argc, argv, "cD:deNPf:v", long_options, &option_index)) != -1)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'c':
@@ -518,7 +521,9 @@ main(int argc, char *argv[])
 				mode = PG_MODE_ENABLE;
 				break;
 			case 'f':
-				if (atoi(optarg) == 0)
+				errno = 0;
+				if (strtoint(optarg, &endptr, 10) == 0
+					|| *endptr || errno == ERANGE)
 				{
 					pg_log_error("invalid filenode specification, must be numeric: %s", optarg);
 					exit(1);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7985da0a94..78882dec48 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2331,6 +2331,8 @@ main(int argc, char **argv)
 	/* process command-line options */
 	while (optind < argc)
 	{
+		char *endptr;
+
 		while ((c = getopt_long(argc, argv, "cD:e:l:m:N:o:p:P:sS:t:U:wW",
 								long_options, &option_index)) != -1)
 		{
@@ -2396,7 +2398,13 @@ main(int argc, char **argv)
 #endif
 					break;
 				case 't':
-					wait_seconds = atoi(optarg);
+					errno = 0;
+					wait_seconds = strtoint(optarg, &endptr, 10);
+					if (*endptr || errno == ERANGE || wait_seconds < 0)
+					{
+						pg_log_error("invalid timeout \"%s\"", optarg);
+						exit(1);
+					}
 					wait_seconds_arg = true;
 					break;
 				case 'U':
@@ -2459,7 +2467,13 @@ main(int argc, char **argv)
 				}
 				ctl_command = KILL_COMMAND;
 				set_sig(argv[++optind]);
-				killproc = atol(argv[++optind]);
+				errno = 0;
+				killproc = strtol(argv[++optind], &endptr, 10);
+				if (*endptr || errno == ERANGE || killproc < 0)
+				{
+					pg_log_error("invalid process ID \"%s\"", argv[optind]);
+					exit(1);
+				}
 			}
 #ifdef WIN32
 			else if (strcmp(argv[optind], "register") == 0)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 321152151d..793f4b3509 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -54,6 +54,7 @@
 #include "catalog/pg_trigger_d.h"
 #include "catalog/pg_type_d.h"
 #include "common/connect.h"
+#include "common/string.h"
 #include "dumputils.h"
 #include "fe_utils/string_utils.h"
 #include "getopt_long.h"
@@ -486,7 +487,19 @@ main(int argc, char **argv)
 				break;
 
 			case 'j':			/* number of dump jobs */
-				numWorkers = atoi(optarg);
+				errno = 0;
+				numWorkers = strtoint(optarg, &endptr, 10);
+				/*
+				 * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS
+				 * (= 64 usually) parallel jobs because that's the maximum
+				 * limit for the WaitForMultipleObjects() call.
+				 */
+				if (*endptr || errno == ERANGE || numWorkers <= 0
+#ifdef WIN32
+					|| numWorkers > MAXIMUM_WAIT_OBJECTS
+#endif
+					)
+					fatal("invalid number of parallel jobs %s", optarg);
 				break;
 
 			case 'n':			/* include schema(s) */
@@ -549,8 +562,10 @@ main(int argc, char **argv)
 				break;
 
 			case 'Z':			/* Compression Level */
-				compressLevel = atoi(optarg);
-				if (compressLevel < 0 || compressLevel > 9)
+				errno = 0;
+				compressLevel = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					compressLevel < 0 || compressLevel > 9)
 				{
 					pg_log_error("compression level must be in range 0..9");
 					exit_nicely(1);
@@ -587,8 +602,10 @@ 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)
+				errno = 0;
+				extra_float_digits = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					extra_float_digits < -15 || extra_float_digits > 3)
 				{
 					pg_log_error("extra_float_digits must be in range -15..3");
 					exit_nicely(1);
@@ -719,18 +736,6 @@ main(int argc, char **argv)
 	if (!plainText)
 		dopt.outputCreateDB = 1;
 
-	/*
-	 * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS (= 64 usually)
-	 * parallel jobs because that's the maximum limit for the
-	 * WaitForMultipleObjects() call.
-	 */
-	if (numWorkers <= 0
-#ifdef WIN32
-		|| numWorkers > MAXIMUM_WAIT_OBJECTS
-#endif
-		)
-		fatal("invalid number of parallel jobs");
-
 	/* Parallel backup only in the directory archive format so far */
 	if (archiveFormat != archDirectory && numWorkers > 1)
 		fatal("parallel backup only supported by the directory format");
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 589b4aed53..285a09aaac 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -39,6 +39,7 @@
  *-------------------------------------------------------------------------
  */
 #include "postgres_fe.h"
+#include "common/string.h"
 
 #include <ctype.h>
 #ifdef HAVE_TERMIOS_H
@@ -151,6 +152,8 @@ 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)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'a':			/* Dump data only */
@@ -181,7 +184,13 @@ main(int argc, char **argv)
 				break;
 
 			case 'j':			/* number of restore jobs */
-				numWorkers = atoi(optarg);
+				errno = 0;
+				numWorkers = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || numWorkers <= 0)
+				{
+					pg_log_error("invalid number of parallel jobs");
+					exit(1);
+				}
 				break;
 
 			case 'l':			/* Dump the TOC summary */
@@ -344,12 +353,6 @@ main(int argc, char **argv)
 		exit_nicely(1);
 	}
 
-	if (numWorkers <= 0)
-	{
-		pg_log_error("invalid number of parallel jobs");
-		exit(1);
-	}
-
 	/* See comments in pg_dump.c */
 #ifdef WIN32
 	if (numWorkers > MAXIMUM_WAIT_OBJECTS)
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 64bbda5650..357ba04f4f 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -104,6 +104,8 @@ 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)
 	{
+		char *endptr;
+
 		switch (option)
 		{
 			case 'b':
@@ -127,7 +129,12 @@ parseCommandLine(int argc, char *argv[])
 				break;
 
 			case 'j':
-				user_opts.jobs = atoi(optarg);
+				errno = 0;
+				user_opts.jobs = strtoint(optarg, &endptr, 10);
+				/**/
+				if (*endptr || errno == ERANGE)
+					pg_fatal("invalid number of jobs %s\n", optarg);
+					
 				break;
 
 			case 'k':
@@ -166,13 +173,17 @@ 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)
-					pg_fatal("invalid old port number\n");
+				errno = 0;
+				if ((old_cluster.port = strtoint(optarg, &endptr, 10)) <= 0 ||
+					*endptr || errno == ERANGE)
+					pg_fatal("invalid old port number %s\n", optarg);
 				break;
 
 			case 'P':
-				if ((new_cluster.port = atoi(optarg)) <= 0)
-					pg_fatal("invalid new port number\n");
+				errno = 0;
+				if ((new_cluster.port = strtoint(optarg, &endptr, 10)) <= 0 ||
+					*endptr || errno == ERANGE)
+					pg_fatal("invalid new port number %s\n", optarg);
 				break;
 
 			case 'r':
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 4aeccd93af..4020347585 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5838,6 +5838,7 @@ 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;
+		char 	   *endptr;
 
 		switch (c)
 		{
@@ -5869,8 +5870,9 @@ main(int argc, char **argv)
 				break;
 			case 'c':
 				benchmarking_option_set = true;
-				nclients = atoi(optarg);
-				if (nclients <= 0)
+				errno = 0;
+				nclients = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || nclients <= 0)
 				{
 					pg_log_fatal("invalid number of clients: \"%s\"", optarg);
 					exit(1);
@@ -5896,8 +5898,9 @@ main(int argc, char **argv)
 				break;
 			case 'j':			/* jobs */
 				benchmarking_option_set = true;
-				nthreads = atoi(optarg);
-				if (nthreads <= 0)
+				errno = 0;
+				nthreads = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || nthreads <= 0)
 				{
 					pg_log_fatal("invalid number of threads: \"%s\"", optarg);
 					exit(1);
@@ -5920,8 +5923,9 @@ main(int argc, char **argv)
 				break;
 			case 's':
 				scale_given = true;
-				scale = atoi(optarg);
-				if (scale <= 0)
+				errno = 0;
+				scale = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || scale <= 0)
 				{
 					pg_log_fatal("invalid scaling factor: \"%s\"", optarg);
 					exit(1);
@@ -5929,8 +5933,9 @@ main(int argc, char **argv)
 				break;
 			case 't':
 				benchmarking_option_set = true;
-				nxacts = atoi(optarg);
-				if (nxacts <= 0)
+				errno = 0;
+				nxacts = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || nxacts <= 0)
 				{
 					pg_log_fatal("invalid number of transactions: \"%s\"", optarg);
 					exit(1);
@@ -5938,8 +5943,9 @@ main(int argc, char **argv)
 				break;
 			case 'T':
 				benchmarking_option_set = true;
-				duration = atoi(optarg);
-				if (duration <= 0)
+				errno = 0;
+				duration = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || duration <= 0)
 				{
 					pg_log_fatal("invalid duration: \"%s\"", optarg);
 					exit(1);
@@ -6001,8 +6007,10 @@ main(int argc, char **argv)
 				break;
 			case 'F':
 				initialization_option_set = true;
-				fillfactor = atoi(optarg);
-				if (fillfactor < 10 || fillfactor > 100)
+				errno = 0;
+				fillfactor = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					fillfactor < 10 || fillfactor > 100)
 				{
 					pg_log_fatal("invalid fillfactor: \"%s\"", optarg);
 					exit(1);
@@ -6021,8 +6029,9 @@ main(int argc, char **argv)
 				break;
 			case 'P':
 				benchmarking_option_set = true;
-				progress = atoi(optarg);
-				if (progress <= 0)
+				errno = 0;
+				progress = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || progress <= 0)
 				{
 					pg_log_fatal("invalid thread progress delay: \"%s\"", optarg);
 					exit(1);
@@ -6031,11 +6040,13 @@ main(int argc, char **argv)
 			case 'R':
 				{
 					/* get a double from the beginning of option value */
-					double		throttle_value = atof(optarg);
+					double		throttle_value;
 
+					errno = 0;
+					throttle_value = strtod(optarg, &endptr);
 					benchmarking_option_set = true;
 
-					if (throttle_value <= 0.0)
+					if (*endptr || errno == ERANGE || throttle_value <= 0.0)
 					{
 						pg_log_fatal("invalid rate limit: \"%s\"", optarg);
 						exit(1);
@@ -6046,9 +6057,12 @@ main(int argc, char **argv)
 				break;
 			case 'L':
 				{
-					double		limit_ms = atof(optarg);
+					double		limit_ms;
 
-					if (limit_ms <= 0.0)
+					errno = 0;
+					limit_ms = strtod(optarg, &endptr);
+
+					if (*endptr || errno == ERANGE || limit_ms <= 0.0)
 					{
 						pg_log_fatal("invalid latency limit: \"%s\"", optarg);
 						exit(1);
@@ -6071,8 +6085,10 @@ main(int argc, char **argv)
 				break;
 			case 4:				/* sampling-rate */
 				benchmarking_option_set = true;
-				sample_rate = atof(optarg);
-				if (sample_rate <= 0.0 || sample_rate > 1.0)
+				errno = 0;
+				sample_rate = strtod(optarg, &endptr);
+				if (*endptr || errno == ERANGE ||
+					sample_rate <= 0.0 || sample_rate > 1.0)
 				{
 					pg_log_fatal("invalid sampling rate: \"%s\"", optarg);
 					exit(1);
@@ -6080,8 +6096,9 @@ main(int argc, char **argv)
 				break;
 			case 5:				/* aggregate-interval */
 				benchmarking_option_set = true;
-				agg_interval = atoi(optarg);
-				if (agg_interval <= 0)
+				errno = 0;
+				agg_interval = strtod(optarg, &endptr);
+				if (*endptr || errno == ERANGE || agg_interval <= 0)
 				{
 					pg_log_fatal("invalid number of seconds for aggregation: \"%s\"", optarg);
 					exit(1);
@@ -6117,8 +6134,9 @@ main(int argc, char **argv)
 				break;
 			case 11:			/* partitions */
 				initialization_option_set = true;
-				partitions = atoi(optarg);
-				if (partitions < 0)
+				errno = 0;
+				partitions = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || partitions < 0)
 				{
 					pg_log_fatal("invalid number of partitions: \"%s\"", optarg);
 					exit(1);
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 543401c6d6..aaed986ae1 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1039,8 +1039,11 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch,
 			}
 			if (ln)
 			{
-				lineno = atoi(ln);
-				if (lineno < 1)
+				char *endptr;
+
+				errno = 0;
+				lineno = strtoint(ln, &endptr, 10);
+				if (*endptr || errno == ERANGE || lineno < 1)
 				{
 					pg_log_error("invalid line number: %s", ln);
 					status = PSQL_CMD_ERROR;
@@ -4283,7 +4286,21 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "border") == 0)
 	{
 		if (value)
-			popt->topt.border = atoi(value);
+		{
+			char *endptr;
+			int new_value;
+
+			errno = 0;
+			new_value = strtoint(value, &endptr, 10);
+			if (*endptr || errno == ERANGE ||
+				new_value < 0 || new_value > 65535)
+			{
+				pg_log_error("\\pset: border is invalid or out of range");
+				return false;
+			}
+
+			popt->topt.border = new_value;
+		}
 	}
 
 	/* set expanded/vertical mode */
@@ -4439,7 +4456,20 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "pager_min_lines") == 0)
 	{
 		if (value)
-			popt->topt.pager_min_lines = atoi(value);
+		{
+			char *endptr;
+			int new_value;
+
+			errno = 0;
+			new_value = strtoint(value, &endptr, 10);
+			if (*endptr || errno == ERANGE || new_value < 0)
+			{
+				pg_log_error("\\pset: pager_min_lines is invalid or out of range");
+				return false;
+			}
+
+			popt->topt.pager_min_lines = new_value;
+		}
 	}
 
 	/* disable "(x rows)" footer */
@@ -4455,7 +4485,19 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "columns") == 0)
 	{
 		if (value)
-			popt->topt.columns = atoi(value);
+		{
+			char *endptr;
+			int new_value;
+
+			errno = 0;
+			new_value = strtoint(value, &endptr, 10);
+			if (*endptr || errno == ERANGE || new_value < 0)
+			{
+				pg_log_error("\\pset: column is invalid or out of range");
+				return false;
+			}
+			popt->topt.columns = new_value;
+		}
 	}
 	else
 	{
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fc0681538a..baa68d58d8 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -15,6 +15,7 @@
 #include "common.h"
 #include "common/connect.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/option_utils.h"
 #include "fe_utils/parallel_slot.h"
@@ -109,6 +110,8 @@ 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)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'h':
@@ -151,10 +154,11 @@ main(int argc, char *argv[])
 				simple_string_list_append(&indexes, optarg);
 				break;
 			case 'j':
-				concurrentCons = atoi(optarg);
-				if (concurrentCons <= 0)
+				errno = 0;
+				concurrentCons = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || concurrentCons <= 0)
 				{
-					pg_log_error("number of parallel jobs must be at least 1");
+					pg_log_error("number of parallel jobs must be at least 1: %s", optarg);
 					exit(1);
 				}
 				break;
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 61974baa78..93b563998a 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -17,6 +17,7 @@
 #include "common.h"
 #include "common/connect.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/option_utils.h"
 #include "fe_utils/parallel_slot.h"
@@ -141,6 +142,8 @@ main(int argc, char *argv[])
 
 	while ((c = getopt_long(argc, argv, "h:p:U:wWeqd:zZFat:fvj:P:", long_options, &optindex)) != -1)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'h':
@@ -192,16 +195,18 @@ main(int argc, char *argv[])
 				vacopts.verbose = true;
 				break;
 			case 'j':
-				concurrentCons = atoi(optarg);
-				if (concurrentCons <= 0)
+				errno = 0;
+				concurrentCons = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || concurrentCons <= 0)
 				{
 					pg_log_error("number of parallel jobs must be at least 1");
 					exit(1);
 				}
 				break;
 			case 'P':
-				vacopts.parallel_workers = atoi(optarg);
-				if (vacopts.parallel_workers < 0)
+				errno = 0;
+				vacopts.parallel_workers = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || vacopts.parallel_workers < 0)
 				{
 					pg_log_error("parallel workers for vacuum must be greater than or equal to zero");
 					exit(1);
@@ -220,16 +225,18 @@ main(int argc, char *argv[])
 				vacopts.skip_locked = true;
 				break;
 			case 6:
-				vacopts.min_xid_age = atoi(optarg);
-				if (vacopts.min_xid_age <= 0)
+				errno = 0;
+				vacopts.min_xid_age = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || vacopts.min_xid_age <= 0)
 				{
 					pg_log_error("minimum transaction ID age must be at least 1");
 					exit(1);
 				}
 				break;
 			case 7:
-				vacopts.min_mxid_age = atoi(optarg);
-				if (vacopts.min_mxid_age <= 0)
+				errno = 0;
+				vacopts.min_mxid_age = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || vacopts.min_mxid_age <= 0)
 				{
 					pg_log_error("minimum multixact ID age must be at least 1");
 					exit(1);
-- 
2.27.0


----Next_Part(Thu_Jul__8_17_30_23_2021_499)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Make-complain-for-invalid-numeirc-values-in-environe.patch"



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH v3 1/3] Be strict in numeric parameters on command line
@ 2021-07-08 06:08 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Kyotaro Horiguchi @ 2021-07-08 06:08 UTC (permalink / raw)

Some numeric command line parameters are tolerant of valid values
followed by garbage like "123xyz".  Be strict to reject such invalid
values. Do the same for psql meta command parameters.
---
 src/bin/pg_amcheck/pg_amcheck.c        |   9 +-
 src/bin/pg_basebackup/pg_basebackup.c  |  17 ++-
 src/bin/pg_basebackup/pg_receivewal.c  |  25 ++--
 src/bin/pg_basebackup/pg_recvlogical.c |  30 ++---
 src/bin/pg_checksums/Makefile          |   4 +-
 src/bin/pg_checksums/pg_checksums.c    |   9 +-
 src/bin/pg_ctl/Makefile                |   4 +-
 src/bin/pg_ctl/pg_ctl.c                |  39 ++++++-
 src/bin/pg_dump/pg_dump.c              |  44 ++++---
 src/bin/pg_dump/pg_restore.c           |  29 ++---
 src/bin/pg_upgrade/option.c            |  24 +++-
 src/bin/pgbench/pgbench.c              | 119 ++++++++++---------
 src/bin/psql/command.c                 |  63 ++++++++--
 src/bin/scripts/reindexdb.c            |   9 +-
 src/bin/scripts/vacuumdb.c             |  31 ++---
 src/fe_utils/option_utils.c            | 152 +++++++++++++++++++++++++
 src/include/common/logging.h           |   9 ++
 src/include/fe_utils/option_utils.h    |  23 ++++
 18 files changed, 457 insertions(+), 183 deletions(-)

diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 4bde16fb4b..538436d4a5 100644
--- a/src/bin/pg_amcheck/pg_amcheck.c
+++ b/src/bin/pg_amcheck/pg_amcheck.c
@@ -12,11 +12,13 @@
  */
 #include "postgres_fe.h"
 
+#include <limits.h>
 #include <time.h>
 
 #include "catalog/pg_am_d.h"
 #include "catalog/pg_namespace_d.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "common/username.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/option_utils.h"
@@ -326,12 +328,9 @@ main(int argc, char *argv[])
 				append_btree_pattern(&opts.exclude, optarg, encoding);
 				break;
 			case 'j':
-				opts.jobs = atoi(optarg);
-				if (opts.jobs < 1)
-				{
-					pg_log_error("number of parallel jobs must be at least 1");
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-j/--jobs",
+									 1, INT_MAX, &opts.jobs))
 					exit(1);
-				}
 				break;
 			case 'p':
 				port = pg_strdup(optarg);
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8bb0acf498..42a65ffbc3 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -15,6 +15,7 @@
 
 #include <unistd.h>
 #include <dirent.h>
+#include <limits.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
 #include <signal.h>
@@ -31,6 +32,7 @@
 #include "common/file_utils.h"
 #include "common/logging.h"
 #include "common/string.h"
+#include "fe_utils/option_utils.h"
 #include "fe_utils/recovery_gen.h"
 #include "fe_utils/string_utils.h"
 #include "getopt_long.h"
@@ -2371,12 +2373,9 @@ main(int argc, char **argv)
 #endif
 				break;
 			case 'Z':
-				compresslevel = atoi(optarg);
-				if (compresslevel < 0 || compresslevel > 9)
-				{
-					pg_log_error("invalid compression level \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-Z/--compress",
+									 0, 9, &compresslevel))
 					exit(1);
-				}
 				break;
 			case 'c':
 				if (pg_strcasecmp(optarg, "fast") == 0)
@@ -2409,12 +2408,10 @@ main(int argc, char **argv)
 				dbgetpassword = 1;
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
-				{
-					pg_log_error("invalid status interval \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_ERROR, optarg,
+									 "-s/--status-interval",
+									 0, INT_MAX, &standby_message_timeout))
 					exit(1);
-				}
 				break;
 			case 'v':
 				verbose++;
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index c1334fad35..1f976ad3eb 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -15,6 +15,7 @@
 #include "postgres_fe.h"
 
 #include <dirent.h>
+#include <limits.h>
 #include <signal.h>
 #include <sys/stat.h>
 #include <unistd.h>
@@ -22,6 +23,8 @@
 #include "access/xlog_internal.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/string.h"
+#include "fe_utils/option_utils.h"
 #include "getopt_long.h"
 #include "libpq-fe.h"
 #include "receivelog.h"
@@ -532,11 +535,10 @@ main(int argc, char **argv)
 				dbhost = pg_strdup(optarg);
 				break;
 			case 'p':
-				if (atoi(optarg) <= 0)
-				{
-					pg_log_error("invalid port number \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-p/--port",
+									 1, 65535, NULL))
 					exit(1);
-				}
+
 				dbport = pg_strdup(optarg);
 				break;
 			case 'U':
@@ -549,12 +551,10 @@ main(int argc, char **argv)
 				dbgetpassword = 1;
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
-				{
-					pg_log_error("invalid status interval \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_ERROR, optarg,
+									 "-s/--status-interval",
+									 0, INT_MAX, &standby_message_timeout))
 					exit(1);
-				}
 				break;
 			case 'S':
 				replication_slot = pg_strdup(optarg);
@@ -574,12 +574,9 @@ main(int argc, char **argv)
 				verbose++;
 				break;
 			case 'Z':
-				compresslevel = atoi(optarg);
-				if (compresslevel < 0 || compresslevel > 9)
-				{
-					pg_log_error("invalid compression level \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-Z/--compress",
+									 0, 9, &compresslevel))
 					exit(1);
-				}
 				break;
 /* action */
 			case 1:
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 76bd153fac..44622c002b 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -13,6 +13,7 @@
 #include "postgres_fe.h"
 
 #include <dirent.h>
+#include <limits.h>
 #include <sys/stat.h>
 #include <unistd.h>
 #ifdef HAVE_SYS_SELECT_H
@@ -23,6 +24,8 @@
 #include "common/fe_memutils.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/string.h"
+#include "fe_utils/option_utils.h"
 #include "getopt_long.h"
 #include "libpq-fe.h"
 #include "libpq/pqsignal.h"
@@ -739,12 +742,12 @@ main(int argc, char **argv)
 				outfile = pg_strdup(optarg);
 				break;
 			case 'F':
-				fsync_interval = atoi(optarg) * 1000;
-				if (fsync_interval < 0)
-				{
-					pg_log_error("invalid fsync interval \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_ERROR, optarg,
+									 "-F/--fsync-interval",
+									 0, INT_MAX / 1000, &fsync_interval))
 					exit(1);
-				}
+				/* convert to milliseconds */
+				fsync_interval *= 1000;
 				break;
 			case 'n':
 				noloop = 1;
@@ -763,11 +766,9 @@ main(int argc, char **argv)
 				dbhost = pg_strdup(optarg);
 				break;
 			case 'p':
-				if (atoi(optarg) <= 0)
-				{
-					pg_log_error("invalid port number \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-p/--port",
+									 0, 65535, NULL))
 					exit(1);
-				}
 				dbport = pg_strdup(optarg);
 				break;
 			case 'U':
@@ -820,12 +821,13 @@ main(int argc, char **argv)
 				plugin = pg_strdup(optarg);
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
-				{
-					pg_log_error("invalid status interval \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_ERROR, optarg,
+									 "-s/--status-interval",
+									 0, INT_MAX / 1000,
+									 &standby_message_timeout))
 					exit(1);
-				}
+				/* convert to milliseconds */
+				standby_message_timeout *= 1000;
 				break;
 			case 'S':
 				replication_slot = pg_strdup(optarg);
diff --git a/src/bin/pg_checksums/Makefile b/src/bin/pg_checksums/Makefile
index ba62406105..a22cf107f9 100644
--- a/src/bin/pg_checksums/Makefile
+++ b/src/bin/pg_checksums/Makefile
@@ -15,13 +15,15 @@ subdir = src/bin/pg_checksums
 top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
+LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils
+
 OBJS = \
 	$(WIN32RES) \
 	pg_checksums.o
 
 all: pg_checksums
 
-pg_checksums: $(OBJS) | submake-libpgport
+pg_checksums: $(OBJS) | submake-libpgport submake-libpgfeutils
 	$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
 
 install: all installdirs
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 3c326906e2..d62df9747a 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -15,6 +15,7 @@
 #include "postgres_fe.h"
 
 #include <dirent.h>
+#include <limits.h>
 #include <time.h>
 #include <sys/stat.h>
 #include <unistd.h>
@@ -24,6 +25,8 @@
 #include "common/file_perm.h"
 #include "common/file_utils.h"
 #include "common/logging.h"
+#include "common/string.h"
+#include "fe_utils/option_utils.h"
 #include "getopt_long.h"
 #include "pg_getopt.h"
 #include "storage/bufpage.h"
@@ -518,11 +521,9 @@ main(int argc, char *argv[])
 				mode = PG_MODE_ENABLE;
 				break;
 			case 'f':
-				if (atoi(optarg) == 0)
-				{
-					pg_log_error("invalid filenode specification, must be numeric: %s", optarg);
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-f/--filenode",
+									 1, INT_MAX, NULL))
 					exit(1);
-				}
 				only_filenode = pstrdup(optarg);
 				break;
 			case 'N':
diff --git a/src/bin/pg_ctl/Makefile b/src/bin/pg_ctl/Makefile
index 5d5f5372a3..ed03d6dc7d 100644
--- a/src/bin/pg_ctl/Makefile
+++ b/src/bin/pg_ctl/Makefile
@@ -24,13 +24,15 @@ LDFLAGS_INTERNAL += $(libpq_pgport)
 SUBMAKE_LIBPQ := submake-libpq
 endif
 
+LDFLAGS_INTERNAL +=  -L$(top_builddir)/src/fe_utils -lpgfeutils
+
 OBJS = \
 	$(WIN32RES) \
 	pg_ctl.o
 
 all: pg_ctl
 
-pg_ctl: $(OBJS) | submake-libpgport $(SUBMAKE_LIBPQ)
+pg_ctl: $(OBJS) | submake-libpgport $(SUBMAKE_LIBPQ) submake-libpgfeutils
 	$(CC) $(CFLAGS) $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
 
 install: all installdirs
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7985da0a94..f7ccdcdc96 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -12,6 +12,7 @@
 #include "postgres_fe.h"
 
 #include <fcntl.h>
+#include <limits.h>
 #include <signal.h>
 #include <time.h>
 #include <sys/stat.h>
@@ -28,6 +29,7 @@
 #include "common/file_perm.h"
 #include "common/logging.h"
 #include "common/string.h"
+#include "fe_utils/option_utils.h"
 #include "getopt_long.h"
 #include "utils/pidfile.h"
 
@@ -76,6 +78,7 @@ typedef enum
 
 #define WAITS_PER_SEC	10		/* should divide USEC_PER_SEC evenly */
 
+static bool do_wait_arg = false;
 static bool do_wait = true;
 static int	wait_seconds = DEFAULT_WAIT;
 static bool wait_seconds_arg = false;
@@ -2396,7 +2399,9 @@ main(int argc, char **argv)
 #endif
 					break;
 				case 't':
-					wait_seconds = atoi(optarg);
+					if (option_parse_int(PG_LOG_ERROR, optarg, "-t/--timeout",
+										 0, INT_MAX, &wait_seconds))
+						exit(1);
 					wait_seconds_arg = true;
 					break;
 				case 'U':
@@ -2408,6 +2413,7 @@ main(int argc, char **argv)
 					break;
 				case 'w':
 					do_wait = true;
+					do_wait_arg = true;
 					break;
 				case 'W':
 					do_wait = false;
@@ -2459,7 +2465,28 @@ main(int argc, char **argv)
 				}
 				ctl_command = KILL_COMMAND;
 				set_sig(argv[++optind]);
-				killproc = atol(argv[++optind]);
+				switch (option_parse_long(PG_LOG_OMIT, argv[++optind], "PID",
+										  0, LONG_MAX, &killproc))
+				{
+					case OPTPARSE_SUCCESS:
+						break;
+					case OPTPARSE_MALFORMED:
+						write_stderr(_("%s: PID must be an integer: \"%s\"\n"),
+									 progname, argv[optind]);
+						exit(1);
+						break;
+					case OPTPARSE_TOOSMALL:
+						write_stderr(_("%s: PID must be a non-negative integer: %s\n"),
+									 progname, argv[optind]);
+						exit(1);
+						break;
+					case OPTPARSE_TOOLARGE:
+					case OPTPARSE_OUTOFRANGE:
+						write_stderr(_("%s: PID out of range: %s\n"),
+									 progname, argv[optind]);
+						exit(1);
+						break;
+				}
 			}
 #ifdef WIN32
 			else if (strcmp(argv[optind], "register") == 0)
@@ -2514,6 +2541,14 @@ main(int argc, char **argv)
 		do_wait = false;
 	}
 
+	if (wait_seconds == 0 && do_wait)
+	{
+		/* Warn if user instructed to wait but we actually don't */
+		if (!silent_mode && do_wait_arg)
+			write_stderr(_("%s: WARNING: -w is ignored because timeout is set to 0\n"), progname);
+		do_wait = false;
+	}
+
 	if (pg_data)
 	{
 		snprintf(postopts_file, MAXPGPATH, "%s/postmaster.opts", pg_data);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 34b91bb226..41b360edc0 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -55,7 +55,9 @@
 #include "catalog/pg_trigger_d.h"
 #include "catalog/pg_type_d.h"
 #include "common/connect.h"
+#include "common/string.h"
 #include "dumputils.h"
+#include "fe_utils/option_utils.h"
 #include "fe_utils/string_utils.h"
 #include "getopt_long.h"
 #include "libpq/libpq-fs.h"
@@ -104,6 +106,17 @@ static Oid	g_last_builtin_oid; /* value of the last builtin oid */
 /* The specified names/patterns should to match at least one entity */
 static int	strict_names = 0;
 
+/*
+ * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS (= 64 usually)
+ * parallel jobs because that's the maximum limit for the
+ * WaitForMultipleObjects() call.
+ */
+#ifndef WIN32
+#define	MAX_NUM_WORKERS INT_MAX
+#else
+#define	MAX_NUM_WORKERS MAXIMUM_WAIT_OBJECTS
+#endif
+
 /*
  * Object inclusion/exclusion lists
  *
@@ -487,7 +500,9 @@ main(int argc, char **argv)
 				break;
 
 			case 'j':			/* number of dump jobs */
-				numWorkers = atoi(optarg);
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-j/--jobs",
+									 1, MAX_NUM_WORKERS, &numWorkers))
+					exit_nicely(1);
 				break;
 
 			case 'n':			/* include schema(s) */
@@ -550,12 +565,9 @@ main(int argc, char **argv)
 				break;
 
 			case 'Z':			/* Compression Level */
-				compressLevel = atoi(optarg);
-				if (compressLevel < 0 || compressLevel > 9)
-				{
-					pg_log_error("compression level must be in range 0..9");
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-Z/--compress",
+									 0, 9, &compressLevel))
 					exit_nicely(1);
-				}
 				break;
 
 			case 0:
@@ -588,12 +600,10 @@ 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)
-				{
-					pg_log_error("extra_float_digits must be in range -15..3");
+				if (option_parse_int(PG_LOG_ERROR, optarg,
+									 "--extra-float-digits",
+									 -15, 3, &extra_float_digits))
 					exit_nicely(1);
-				}
 				break;
 
 			case 9:				/* inserts */
@@ -720,18 +730,6 @@ main(int argc, char **argv)
 	if (!plainText)
 		dopt.outputCreateDB = 1;
 
-	/*
-	 * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS (= 64 usually)
-	 * parallel jobs because that's the maximum limit for the
-	 * WaitForMultipleObjects() call.
-	 */
-	if (numWorkers <= 0
-#ifdef WIN32
-		|| numWorkers > MAXIMUM_WAIT_OBJECTS
-#endif
-		)
-		fatal("invalid number of parallel jobs");
-
 	/* Parallel backup only in the directory archive format so far */
 	if (archiveFormat != archDirectory && numWorkers > 1)
 		fatal("parallel backup only supported by the directory format");
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 589b4aed53..6f4447027a 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -39,6 +39,8 @@
  *-------------------------------------------------------------------------
  */
 #include "postgres_fe.h"
+#include "common/string.h"
+#include "fe_utils/option_utils.h"
 
 #include <ctype.h>
 #ifdef HAVE_TERMIOS_H
@@ -52,6 +54,13 @@
 
 static void usage(const char *progname);
 
+/* See comments in pg_dump.c */
+#ifndef WIN32
+#define MAX_NUM_WORKERS INT_MAX
+#else
+#define MAX_NUM_WORKERS MAXIMUM_WAIT_OBJECTS
+#endif
+
 int
 main(int argc, char **argv)
 {
@@ -181,7 +190,9 @@ main(int argc, char **argv)
 				break;
 
 			case 'j':			/* number of restore jobs */
-				numWorkers = atoi(optarg);
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-j/--jobs",
+									 1, MAX_NUM_WORKERS, &numWorkers))
+					exit(1);
 				break;
 
 			case 'l':			/* Dump the TOC summary */
@@ -344,22 +355,6 @@ main(int argc, char **argv)
 		exit_nicely(1);
 	}
 
-	if (numWorkers <= 0)
-	{
-		pg_log_error("invalid number of parallel jobs");
-		exit(1);
-	}
-
-	/* See comments in pg_dump.c */
-#ifdef WIN32
-	if (numWorkers > MAXIMUM_WAIT_OBJECTS)
-	{
-		pg_log_error("maximum number of parallel jobs is %d",
-					 MAXIMUM_WAIT_OBJECTS);
-		exit(1);
-	}
-#endif
-
 	/* Can't do single-txn mode with multiple connections */
 	if (opts->single_txn && numWorkers > 1)
 	{
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 64bbda5650..a8cd23f7fa 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -9,12 +9,15 @@
 
 #include "postgres_fe.h"
 
+#include <limits.h>
 #include <time.h>
 #ifdef WIN32
 #include <io.h>
 #endif
 
+#include "common/logging.h"
 #include "common/string.h"
+#include "fe_utils/option_utils.h"
 #include "getopt_long.h"
 #include "pg_upgrade.h"
 #include "utils/pidfile.h"
@@ -104,6 +107,8 @@ 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)
 	{
+		int p;
+
 		switch (option)
 		{
 			case 'b':
@@ -127,7 +132,10 @@ parseCommandLine(int argc, char *argv[])
 				break;
 
 			case 'j':
-				user_opts.jobs = atoi(optarg);
+				if (option_parse_int(PG_LOG_OMIT, optarg, "-j/--jobs",
+									 0, INT_MAX, &user_opts.jobs))
+					pg_fatal("-j/--jobs must be an integer in range %d..%d: \"%s\"\n",
+							 0, INT_MAX, optarg);
 				break;
 
 			case 'k':
@@ -166,13 +174,19 @@ 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)
-					pg_fatal("invalid old port number\n");
+				if (option_parse_int(PG_LOG_OMIT, optarg, "-p/--old-port",
+									 1, 65535, &p))
+					pg_fatal("old port number must be an integer in range %d..%d: \"%s\"\n",
+							 1, 65535, optarg);
+				old_cluster.port = p;
 				break;
 
 			case 'P':
-				if ((new_cluster.port = atoi(optarg)) <= 0)
-					pg_fatal("invalid new port number\n");
+				if (option_parse_int(PG_LOG_OMIT, optarg, "-P/--new-port",
+									 1, 65535, &p))
+					pg_fatal("new port number must be an integer in range %d..%d: \"%s\"\n",
+							 1, 65535, optarg);
+				new_cluster.port = p;
 				break;
 
 			case 'r':
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 364b5a2e47..faa0969d70 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -63,6 +63,7 @@
 #include "common/username.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/conditional.h"
+#include "fe_utils/option_utils.h"
 #include "fe_utils/string_utils.h"
 #include "getopt_long.h"
 #include "libpq-fe.h"
@@ -871,7 +872,12 @@ invalid_syntax:
 	return false;
 }
 
-/* convert string to double, detecting overflows/underflows */
+/*
+ * convert string to double, detecting overflows/underflows
+ *
+ * This is similar to option_parse_double but leave this function for
+ * performance reasons.
+ */
 bool
 strtodouble(const char *str, bool errorOK, double *dv)
 {
@@ -5887,12 +5893,9 @@ main(int argc, char **argv)
 				break;
 			case 'c':
 				benchmarking_option_set = true;
-				nclients = atoi(optarg);
-				if (nclients <= 0)
-				{
-					pg_log_fatal("invalid number of clients: \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_FATAL, optarg, "-c/--client",
+									 1, INT_MAX, &nclients))
 					exit(1);
-				}
 #ifdef HAVE_GETRLIMIT
 #ifdef RLIMIT_NOFILE			/* most platforms use RLIMIT_NOFILE */
 				if (getrlimit(RLIMIT_NOFILE, &rlim) == -1)
@@ -5914,12 +5917,10 @@ main(int argc, char **argv)
 				break;
 			case 'j':			/* jobs */
 				benchmarking_option_set = true;
-				nthreads = atoi(optarg);
-				if (nthreads <= 0)
-				{
-					pg_log_fatal("invalid number of threads: \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_FATAL, optarg, "-j/--jobs",
+									 0, INT_MAX, &nthreads))
 					exit(1);
-				}
+
 #ifndef ENABLE_THREAD_SAFETY
 				if (nthreads != 1)
 				{
@@ -5938,30 +5939,21 @@ main(int argc, char **argv)
 				break;
 			case 's':
 				scale_given = true;
-				scale = atoi(optarg);
-				if (scale <= 0)
-				{
-					pg_log_fatal("invalid scaling factor: \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_FATAL, optarg, "-s/--scale",
+									 1, INT_MAX, &scale))
 					exit(1);
-				}
 				break;
 			case 't':
 				benchmarking_option_set = true;
-				nxacts = atoi(optarg);
-				if (nxacts <= 0)
-				{
-					pg_log_fatal("invalid number of transactions: \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_FATAL, optarg, "-t/--transactions",
+									 1, INT_MAX, &nxacts))
 					exit(1);
-				}
 				break;
 			case 'T':
 				benchmarking_option_set = true;
-				duration = atoi(optarg);
-				if (duration <= 0)
-				{
-					pg_log_fatal("invalid duration: \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_FATAL, optarg, "-T/--time",
+									 1, INT_MAX, &duration))
 					exit(1);
-				}
 				break;
 			case 'U':
 				username = pg_strdup(optarg);
@@ -6019,12 +6011,9 @@ main(int argc, char **argv)
 				break;
 			case 'F':
 				initialization_option_set = true;
-				fillfactor = atoi(optarg);
-				if (fillfactor < 10 || fillfactor > 100)
-				{
-					pg_log_fatal("invalid fillfactor: \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_FATAL, optarg, "-F/--fillfactor",
+									 10, 100, &fillfactor))
 					exit(1);
-				}
 				break;
 			case 'M':
 				benchmarking_option_set = true;
@@ -6039,38 +6028,61 @@ main(int argc, char **argv)
 				break;
 			case 'P':
 				benchmarking_option_set = true;
-				progress = atoi(optarg);
-				if (progress <= 0)
-				{
-					pg_log_fatal("invalid thread progress delay: \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_FATAL, optarg, "-P/--progress",
+									 1, INT_MAX, &progress))
 					exit(1);
-				}
 				break;
 			case 'R':
 				{
-					/* get a double from the beginning of option value */
-					double		throttle_value = atof(optarg);
+					double		throttle_value;
 
-					benchmarking_option_set = true;
+					if (option_parse_double(PG_LOG_FATAL, optarg, "-R/--rate",
+											&throttle_value))
+						exit(1);
 
-					if (throttle_value <= 0.0)
+					/*
+					 * This value will be used for yielding int64 number
+					 * through some arithmetic. Quite arbitrary but people
+					 * never want such small numbers for this parameter.
+					 */
+					if (throttle_value <= 1e-5)
 					{
-						pg_log_fatal("invalid rate limit: \"%s\"", optarg);
+						pg_log_fatal("-R/--rate must be greater than 1e-5: \"%s\"", optarg);
 						exit(1);
 					}
+
 					/* Invert rate limit into per-transaction delay in usec */
 					throttle_delay = 1000000.0 / throttle_value;
 				}
 				break;
 			case 'L':
 				{
-					double		limit_ms = atof(optarg);
+					double		limit_ms;
 
+					if (option_parse_double(PG_LOG_FATAL, optarg,
+											"-L/--latency-limit", &limit_ms))
+						exit(1);
+
+					/* limit limit_ms so that latency_limit fits in int64 */
 					if (limit_ms <= 0.0)
 					{
-						pg_log_fatal("invalid latency limit: \"%s\"", optarg);
+						pg_log_fatal("-L/--latency-limit must be greater than zero: \"%s\"",
+									 optarg);
 						exit(1);
 					}
+					/*
+					 * limit_ms * 1000 must fit int64. We could calculate the
+					 * precise limit but also we don't need to accept such a
+					 * large number here.  Thus use a quite arbitrary seconds
+					 * for the limit.
+					 */
+					if (limit_ms > 3600)
+					{
+						pg_log_fatal("-L/--latency-limit must be less than 3600 seconds: \"%s\"",
+									 optarg);
+						exit(1);
+					}
+
 					benchmarking_option_set = true;
 					latency_limit = (int64) (limit_ms * 1000);
 				}
@@ -6089,21 +6101,21 @@ main(int argc, char **argv)
 				break;
 			case 4:				/* sampling-rate */
 				benchmarking_option_set = true;
-				sample_rate = atof(optarg);
+				if (option_parse_double(PG_LOG_FATAL, optarg, "--samplig-rate",
+										&sample_rate))
+					exit(1);
 				if (sample_rate <= 0.0 || sample_rate > 1.0)
 				{
-					pg_log_fatal("invalid sampling rate: \"%s\"", optarg);
+					pg_log_fatal("sampling rate must be greater than zero and less than or equal to 1.0: \"%s\"", optarg);
 					exit(1);
 				}
 				break;
 			case 5:				/* aggregate-interval */
 				benchmarking_option_set = true;
-				agg_interval = atoi(optarg);
-				if (agg_interval <= 0)
-				{
-					pg_log_fatal("invalid number of seconds for aggregation: \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_FATAL, optarg,
+									 "--aggregate-interval",
+									 1, INT_MAX, &agg_interval))
 					exit(1);
-				}
 				break;
 			case 6:				/* progress-timestamp */
 				progress_timestamp = true;
@@ -6135,12 +6147,9 @@ main(int argc, char **argv)
 				break;
 			case 11:			/* partitions */
 				initialization_option_set = true;
-				partitions = atoi(optarg);
-				if (partitions < 0)
-				{
-					pg_log_fatal("invalid number of partitions: \"%s\"", optarg);
+				if (option_parse_int(PG_LOG_FATAL, optarg, "--partitions",
+									  0, INT_MAX, &partitions))
 					exit(1);
-				}
 				break;
 			case 12:			/* partition-method */
 				initialization_option_set = true;
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 49d4c0e3ce..312a797e78 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -11,6 +11,7 @@
 #include <time.h>
 #include <pwd.h>
 #include <utime.h>
+#include <limits.h>
 #ifndef WIN32
 #include <sys/stat.h>			/* for stat() */
 #include <sys/time.h>			/* for setitimer() */
@@ -34,6 +35,7 @@
 #include "describe.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/print.h"
+#include "fe_utils/option_utils.h"
 #include "fe_utils/string_utils.h"
 #include "help.h"
 #include "input.h"
@@ -1040,11 +1042,19 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch,
 			}
 			if (ln)
 			{
-				lineno = atoi(ln);
-				if (lineno < 1)
+				switch (option_parse_int(PG_LOG_OMIT, ln, "line number",
+										 1, INT_MAX, &lineno))
 				{
-					pg_log_error("invalid line number: %s", ln);
-					status = PSQL_CMD_ERROR;
+					case OPTPARSE_SUCCESS:
+						break;
+					case OPTPARSE_MALFORMED:
+					case OPTPARSE_TOOSMALL:
+					case OPTPARSE_TOOLARGE:
+						pg_log_error("line number must be an integer greater than zero: %s", ln);
+						break;
+					case OPTPARSE_OUTOFRANGE:
+						pg_log_error("line number out of range: %s", ln);
+						break;
 				}
 			}
 			if (status != PSQL_CMD_ERROR)
@@ -4284,7 +4294,16 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "border") == 0)
 	{
 		if (value)
-			popt->topt.border = atoi(value);
+		{
+			int v;
+
+			if (option_parse_int(PG_LOG_OMIT, value, "border", 0, 65535, &v))
+			{
+				pg_log_error("\\pset: border must be an integer in range 0..65535: \"%s\"", value);
+				return false;
+			}
+			popt->topt.border = v;
+		}
 	}
 
 	/* set expanded/vertical mode */
@@ -4440,7 +4459,22 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "pager_min_lines") == 0)
 	{
 		if (value)
-			popt->topt.pager_min_lines = atoi(value);
+		{
+			switch (option_parse_int(PG_LOG_OMIT, value, "page_min_lines",
+									 0, INT_MAX, &popt->topt.pager_min_lines))
+			{
+				case OPTPARSE_SUCCESS:
+					break;
+				case OPTPARSE_MALFORMED:
+				case OPTPARSE_TOOSMALL:
+				case OPTPARSE_TOOLARGE:
+					pg_log_error("\\pset: pager_min_lines must be a non-negative integer: \"%s\"", value);
+					return false;
+				case OPTPARSE_OUTOFRANGE:
+					pg_log_error("\\pset: pager_min_lines out of range: \"%s\"", value);
+					return false;
+			}
+		}
 	}
 
 	/* disable "(x rows)" footer */
@@ -4456,7 +4490,22 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "columns") == 0)
 	{
 		if (value)
-			popt->topt.columns = atoi(value);
+		{
+			switch (option_parse_int(PG_LOG_OMIT, value, "columns",
+									 0, INT_MAX, &popt->topt.columns))
+			{
+				case OPTPARSE_SUCCESS:
+					break;
+				case OPTPARSE_MALFORMED:
+				case OPTPARSE_TOOSMALL:
+				case OPTPARSE_TOOLARGE:
+					pg_log_error("\\pset: column must be a non-negative integer: \"%s\"", value);
+					return false;
+				case OPTPARSE_OUTOFRANGE:
+					pg_log_error("\\pset: column out of range: \"%s\"", value);
+					return false;
+			}
+		}
 	}
 	else
 	{
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fc0681538a..f510a63157 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -9,6 +9,8 @@
  *-------------------------------------------------------------------------
  */
 
+#include <limits.h>
+
 #include "postgres_fe.h"
 
 #include "catalog/pg_class_d.h"
@@ -151,12 +153,9 @@ main(int argc, char *argv[])
 				simple_string_list_append(&indexes, optarg);
 				break;
 			case 'j':
-				concurrentCons = atoi(optarg);
-				if (concurrentCons <= 0)
-				{
-					pg_log_error("number of parallel jobs must be at least 1");
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-j/--jobs",
+									 1, INT_MAX, &concurrentCons))
 					exit(1);
-				}
 				break;
 			case 'v':
 				verbose = true;
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index a85919c5c1..21966ed7e2 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -10,6 +10,8 @@
  *-------------------------------------------------------------------------
  */
 
+#include <limits.h>
+
 #include "postgres_fe.h"
 
 #include "catalog/pg_class_d.h"
@@ -17,6 +19,7 @@
 #include "common.h"
 #include "common/connect.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/option_utils.h"
 #include "fe_utils/parallel_slot.h"
@@ -192,20 +195,14 @@ main(int argc, char *argv[])
 				vacopts.verbose = true;
 				break;
 			case 'j':
-				concurrentCons = atoi(optarg);
-				if (concurrentCons <= 0)
-				{
-					pg_log_error("number of parallel jobs must be at least 1");
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-j/--jobs",
+									 1, INT_MAX, &concurrentCons))
 					exit(1);
-				}
 				break;
 			case 'P':
-				vacopts.parallel_workers = atoi(optarg);
-				if (vacopts.parallel_workers < 0)
-				{
-					pg_log_error("parallel workers for vacuum must be greater than or equal to zero");
+				if (option_parse_int(PG_LOG_ERROR, optarg, "-P/--parallel",
+									 0, INT_MAX, &vacopts.parallel_workers))
 					exit(1);
-				}
 				break;
 			case 2:
 				maintenance_db = pg_strdup(optarg);
@@ -220,20 +217,14 @@ main(int argc, char *argv[])
 				vacopts.skip_locked = true;
 				break;
 			case 6:
-				vacopts.min_xid_age = atoi(optarg);
-				if (vacopts.min_xid_age <= 0)
-				{
-					pg_log_error("minimum transaction ID age must be at least 1");
+				if (option_parse_int(PG_LOG_ERROR, optarg, "--min-xid-age",
+									 1, INT_MAX, &vacopts.min_xid_age))
 					exit(1);
-				}
 				break;
 			case 7:
-				vacopts.min_mxid_age = atoi(optarg);
-				if (vacopts.min_mxid_age <= 0)
-				{
-					pg_log_error("minimum multixact ID age must be at least 1");
+				if (option_parse_int(PG_LOG_ERROR, optarg, "--min-mxid-age",
+									 1, INT_MAX, &vacopts.min_mxid_age))
 					exit(1);
-				}
 				break;
 			case 8:
 				vacopts.no_index_cleanup = true;
diff --git a/src/fe_utils/option_utils.c b/src/fe_utils/option_utils.c
index e19a495dba..4f191d321a 100644
--- a/src/fe_utils/option_utils.c
+++ b/src/fe_utils/option_utils.c
@@ -10,8 +10,12 @@
  *-------------------------------------------------------------------------
  */
 
+#include <limits.h>
+
 #include "postgres_fe.h"
 
+#include "common/string.h"
+#include "common/logging.h"
 #include "fe_utils/option_utils.h"
 
 /*
@@ -36,3 +40,151 @@ handle_help_version_opts(int argc, char *argv[],
 		}
 	}
 }
+
+/*
+ * parser for int/long values in decimal for command line options
+ *
+ * Returns OPTPARSE_SUCCESS (0) when successfully parsed.
+ *
+ * Otherwise returns non-zero status numbers indicating error details after
+ * printing a generic error message with the given errorlevel, which may be
+ * omitted when the errorlevel is lower than the common/logging.c's
+ * setting. Giving PG_LOG_OMIT as loglevel disables generic errors.
+ *
+ * If result is not NULL, the result is stored there only when successfully
+ * parsed.
+ */
+optparse_result
+option_parse_int(int loglevel, const char *optarg, const char *optname,
+				 int min_range, int max_range, int *result)
+{
+	char   *endptr;
+	int		parsed;
+
+	errno = 0;
+	parsed = strtoint(optarg, &endptr, 10);
+
+	if (*endptr)
+	{
+		pg_log_level(loglevel, "%s must be an integer: \"%s\"",
+					 optname, optarg);
+		return OPTPARSE_MALFORMED;
+	}
+	else if (errno == ERANGE || parsed < min_range || parsed > max_range)
+	{
+		if (max_range == INT_MAX)
+		{
+			if (errno == ERANGE)
+				pg_log_level(loglevel, "%s out of range: %s",
+							 optname, optarg);
+			else
+				pg_log_level(loglevel, "%s must be at least %d: %s",
+							 optname, min_range, optarg);
+		}
+		else
+			pg_log_level(loglevel, "%s must be in range %d..%d: %s",
+						 optname, min_range, max_range, optarg);
+
+		if (errno == ERANGE)
+			return OPTPARSE_OUTOFRANGE;
+		if (parsed < min_range)
+			return OPTPARSE_TOOSMALL;
+
+		Assert (parsed > max_range);
+		return OPTPARSE_TOOLARGE;
+	}
+
+	if (result)
+		*result = parsed;
+
+	return OPTPARSE_SUCCESS;
+}
+
+
+optparse_result
+option_parse_long(int loglevel, const char *optarg, const char *optname,
+				  long min_range, long max_range, long *result)
+{
+	char   *endptr;
+	long	parsed;
+
+	errno = 0;
+	parsed = strtol(optarg, &endptr, 10);
+
+	if (*endptr)
+	{
+		pg_log_level(loglevel, "%s must be an integer: \"%s\"",
+					 optname, optarg);
+		return OPTPARSE_MALFORMED;
+	}
+	else if (errno == ERANGE || parsed < min_range || parsed > max_range)
+	{
+		if (max_range == LONG_MAX)
+		{
+			if (errno == ERANGE)
+				pg_log_level(loglevel, "%s out of range: %s",
+							 optname, optarg);
+			else
+				pg_log_level(loglevel, "%s must be at least %ld: %s",
+							 optname, min_range, optarg);
+		}
+		else
+			pg_log_level(loglevel, "%s must be in range %ld..%ld: %s",
+						 optname, min_range, max_range, optarg);
+
+		if (errno == ERANGE)
+			return OPTPARSE_OUTOFRANGE;
+		if (parsed < min_range)
+			return OPTPARSE_TOOSMALL;
+
+		Assert (parsed > max_range);
+		return OPTPARSE_TOOLARGE;
+	}
+
+	if (result)
+		*result = parsed;
+
+	return OPTPARSE_SUCCESS;
+}
+
+
+/*
+ * parser for double value in decimal for command line options
+ *
+ * Returns OPTPARSE_SUCCESS (0) when successfully parsed.
+ *
+ * Otherwise returns non-zero status numbers indicating error details after
+ * printing a fixed-form error message with the given errorlevel, which may be
+ * omitted when the errorlevel is lower than the common/logging.c's
+ * setting. Giving PG_LOG_OMIT as loglevel disables this error output.
+ *
+ * If result is not NULL, the result is stored there only when successfully
+ * parsed.
+ */
+optparse_result
+option_parse_double(int loglevel, const char *optarg, const char *optname,
+					double *result)
+{
+	char   *endptr;
+	double	parsed;
+
+	errno = 0;
+	parsed = strtod(optarg, &endptr);
+
+	if (*endptr)
+	{
+		pg_log_level(loglevel, "%s must be a numeric: \"%s\"", optname, optarg);
+		return OPTPARSE_MALFORMED;
+	}
+	else if (errno == ERANGE)
+	{
+		pg_log_level(loglevel, "%s out of range: %s", optname, optarg);
+		
+		return OPTPARSE_OUTOFRANGE;
+	}
+
+	if (result)
+		*result = parsed;
+
+	return OPTPARSE_SUCCESS;
+}
diff --git a/src/include/common/logging.h b/src/include/common/logging.h
index a71cf84249..c2e9d1e46f 100644
--- a/src/include/common/logging.h
+++ b/src/include/common/logging.h
@@ -15,6 +15,11 @@
  */
 enum pg_log_level
 {
+	/*
+	 * Don't show this message
+	 */
+	PG_LOG_OMIT = -1,
+
 	/*
 	 * Not initialized yet
 	 */
@@ -93,4 +98,8 @@ void		pg_log_generic_v(enum pg_log_level level, const char *pg_restrict fmt, va_
 		if (unlikely(__pg_log_level <= PG_LOG_DEBUG)) pg_log_generic(PG_LOG_DEBUG, __VA_ARGS__); \
 	} while(0)
 
+#define pg_log_level(loglevel, ...) do { \
+		if (unlikely(__pg_log_level <= loglevel)) pg_log_generic(loglevel, __VA_ARGS__); \
+	} while(0)
+
 #endif							/* COMMON_LOGGING_H */
diff --git a/src/include/fe_utils/option_utils.h b/src/include/fe_utils/option_utils.h
index d653cb94e3..bfba7a73ed 100644
--- a/src/include/fe_utils/option_utils.h
+++ b/src/include/fe_utils/option_utils.h
@@ -14,10 +14,33 @@
 
 #include "postgres_fe.h"
 
+typedef enum optparse_result
+{
+	OPTPARSE_SUCCESS = 0,
+	OPTPARSE_MALFORMED,
+	OPTPARSE_TOOSMALL,
+	OPTPARSE_TOOLARGE,
+	OPTPARSE_OUTOFRANGE
+} optparse_result;
+
 typedef void (*help_handler) (const char *progname);
 
 extern void handle_help_version_opts(int argc, char *argv[],
 									 const char *fixed_progname,
 									 help_handler hlp);
 
+extern optparse_result option_parse_int(int loglevel,
+										const char *optarg, const char *optname,
+										int min_range, int max_range,
+										int *result);
+
+extern optparse_result option_parse_long(int loglevel,
+										 const char *optarg,
+										 const char *optname,
+										 long min_range, long max_range,
+										 long *result);
+
+extern optparse_result option_parse_double(int loglevel, const char *optarg,
+										   const char *optname, double *result);
+
 #endif							/* OPTION_UTILS_H */
-- 
2.27.0


----Next_Part(Wed_Jul_21_17_02_29_2021_399)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v3-0002-Make-complain-for-invalid-numeirc-values-in-envir.patch"



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH v2 1/2] Be strict in numeric parameters on command line
@ 2021-07-08 06:08 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Kyotaro Horiguchi @ 2021-07-08 06:08 UTC (permalink / raw)

Some numeric command line parameters are tolerant of valid values
followed by garbage like "123xyz".  Be strict to reject such invalid
values. Do the same for psql meta command parameters.
---
 src/bin/pg_amcheck/pg_amcheck.c        |  6 ++-
 src/bin/pg_basebackup/pg_basebackup.c  | 13 +++--
 src/bin/pg_basebackup/pg_receivewal.c  | 18 +++++--
 src/bin/pg_basebackup/pg_recvlogical.c | 17 +++++--
 src/bin/pg_checksums/pg_checksums.c    |  7 ++-
 src/bin/pg_ctl/pg_ctl.c                | 18 ++++++-
 src/bin/pg_dump/pg_dump.c              | 39 ++++++++-------
 src/bin/pg_dump/pg_restore.c           | 17 ++++---
 src/bin/pg_upgrade/option.c            | 21 ++++++--
 src/bin/pgbench/pgbench.c              | 66 ++++++++++++++++----------
 src/bin/psql/command.c                 | 52 ++++++++++++++++++--
 src/bin/scripts/reindexdb.c            | 10 ++--
 src/bin/scripts/vacuumdb.c             | 23 +++++----
 13 files changed, 219 insertions(+), 88 deletions(-)

diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 4bde16fb4b..71a82f9b75 100644
--- a/src/bin/pg_amcheck/pg_amcheck.c
+++ b/src/bin/pg_amcheck/pg_amcheck.c
@@ -17,6 +17,7 @@
 #include "catalog/pg_am_d.h"
 #include "catalog/pg_namespace_d.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "common/username.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/option_utils.h"
@@ -326,8 +327,9 @@ main(int argc, char *argv[])
 				append_btree_pattern(&opts.exclude, optarg, encoding);
 				break;
 			case 'j':
-				opts.jobs = atoi(optarg);
-				if (opts.jobs < 1)
+				errno = 0;
+				opts.jobs = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || opts.jobs < 1)
 				{
 					pg_log_error("number of parallel jobs must be at least 1");
 					exit(1);
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8bb0acf498..29be95b96a 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2287,6 +2287,8 @@ 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)
 	{
+		char   *endptr;
+
 		switch (c)
 		{
 			case 'C':
@@ -2371,8 +2373,10 @@ main(int argc, char **argv)
 #endif
 				break;
 			case 'Z':
-				compresslevel = atoi(optarg);
-				if (compresslevel < 0 || compresslevel > 9)
+				errno = 0;
+				compresslevel = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					compresslevel < 0 || compresslevel > 9)
 				{
 					pg_log_error("invalid compression level \"%s\"", optarg);
 					exit(1);
@@ -2409,8 +2413,9 @@ main(int argc, char **argv)
 				dbgetpassword = 1;
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				errno = 0;
+				standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr || errno == ERANGE || standby_message_timeout < 0)
 				{
 					pg_log_error("invalid status interval \"%s\"", optarg);
 					exit(1);
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index c1334fad35..7fef925b99 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -22,6 +22,7 @@
 #include "access/xlog_internal.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "getopt_long.h"
 #include "libpq-fe.h"
 #include "receivelog.h"
@@ -520,6 +521,8 @@ 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)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'D':
@@ -532,7 +535,9 @@ main(int argc, char **argv)
 				dbhost = pg_strdup(optarg);
 				break;
 			case 'p':
-				if (atoi(optarg) <= 0)
+				errno = 0;
+				if (strtoint(optarg, &endptr, 10) <= 0 ||
+					*endptr || errno == ERANGE)
 				{
 					pg_log_error("invalid port number \"%s\"", optarg);
 					exit(1);
@@ -549,8 +554,9 @@ main(int argc, char **argv)
 				dbgetpassword = 1;
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				errno = 0;
+				standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr || errno == ERANGE || standby_message_timeout < 0)
 				{
 					pg_log_error("invalid status interval \"%s\"", optarg);
 					exit(1);
@@ -574,8 +580,10 @@ main(int argc, char **argv)
 				verbose++;
 				break;
 			case 'Z':
-				compresslevel = atoi(optarg);
-				if (compresslevel < 0 || compresslevel > 9)
+				errno = 0;
+				compresslevel = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					compresslevel < 0 || compresslevel > 9)
 				{
 					pg_log_error("invalid compression level \"%s\"", optarg);
 					exit(1);
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 76bd153fac..7be932d025 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -23,6 +23,7 @@
 #include "common/fe_memutils.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "getopt_long.h"
 #include "libpq-fe.h"
 #include "libpq/pqsignal.h"
@@ -732,6 +733,8 @@ main(int argc, char **argv)
 	while ((c = getopt_long(argc, argv, "E:f:F:nvtd:h:p:U:wWI:o:P:s:S:",
 							long_options, &option_index)) != -1)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 /* general options */
@@ -739,8 +742,9 @@ main(int argc, char **argv)
 				outfile = pg_strdup(optarg);
 				break;
 			case 'F':
-				fsync_interval = atoi(optarg) * 1000;
-				if (fsync_interval < 0)
+				errno = 0;
+				fsync_interval = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr || errno == ERANGE || fsync_interval < 0)
 				{
 					pg_log_error("invalid fsync interval \"%s\"", optarg);
 					exit(1);
@@ -763,7 +767,9 @@ main(int argc, char **argv)
 				dbhost = pg_strdup(optarg);
 				break;
 			case 'p':
-				if (atoi(optarg) <= 0)
+				errno = 0;
+				if (strtoint(optarg, &endptr, 10) <= 0 ||
+					*endptr || errno == ERANGE)
 				{
 					pg_log_error("invalid port number \"%s\"", optarg);
 					exit(1);
@@ -820,8 +826,9 @@ main(int argc, char **argv)
 				plugin = pg_strdup(optarg);
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				errno = 0;
+				standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr || errno == ERANGE || standby_message_timeout < 0)
 				{
 					pg_log_error("invalid status interval \"%s\"", optarg);
 					exit(1);
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 3c326906e2..1c4e5b9d85 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -24,6 +24,7 @@
 #include "common/file_perm.h"
 #include "common/file_utils.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "getopt_long.h"
 #include "pg_getopt.h"
 #include "storage/bufpage.h"
@@ -506,6 +507,8 @@ main(int argc, char *argv[])
 
 	while ((c = getopt_long(argc, argv, "cD:deNPf:v", long_options, &option_index)) != -1)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'c':
@@ -518,7 +521,9 @@ main(int argc, char *argv[])
 				mode = PG_MODE_ENABLE;
 				break;
 			case 'f':
-				if (atoi(optarg) == 0)
+				errno = 0;
+				if (strtoint(optarg, &endptr, 10) == 0
+					|| *endptr || errno == ERANGE)
 				{
 					pg_log_error("invalid filenode specification, must be numeric: %s", optarg);
 					exit(1);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7985da0a94..d6a39182cf 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2331,6 +2331,8 @@ main(int argc, char **argv)
 	/* process command-line options */
 	while (optind < argc)
 	{
+		char *endptr;
+
 		while ((c = getopt_long(argc, argv, "cD:e:l:m:N:o:p:P:sS:t:U:wW",
 								long_options, &option_index)) != -1)
 		{
@@ -2396,7 +2398,13 @@ main(int argc, char **argv)
 #endif
 					break;
 				case 't':
-					wait_seconds = atoi(optarg);
+					errno = 0;
+					wait_seconds = strtoint(optarg, &endptr, 10);
+					if (*endptr || errno == ERANGE || wait_seconds < 1)
+					{
+						pg_log_error("invalid timeout value \"%s\", use --no-wait to finish without waiting", optarg);
+						exit(1);
+					}
 					wait_seconds_arg = true;
 					break;
 				case 'U':
@@ -2459,7 +2467,13 @@ main(int argc, char **argv)
 				}
 				ctl_command = KILL_COMMAND;
 				set_sig(argv[++optind]);
-				killproc = atol(argv[++optind]);
+				errno = 0;
+				killproc = strtol(argv[++optind], &endptr, 10);
+				if (*endptr || errno == ERANGE || killproc < 0)
+				{
+					pg_log_error("invalid process ID \"%s\"", argv[optind]);
+					exit(1);
+				}
 			}
 #ifdef WIN32
 			else if (strcmp(argv[optind], "register") == 0)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 321152151d..793f4b3509 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -54,6 +54,7 @@
 #include "catalog/pg_trigger_d.h"
 #include "catalog/pg_type_d.h"
 #include "common/connect.h"
+#include "common/string.h"
 #include "dumputils.h"
 #include "fe_utils/string_utils.h"
 #include "getopt_long.h"
@@ -486,7 +487,19 @@ main(int argc, char **argv)
 				break;
 
 			case 'j':			/* number of dump jobs */
-				numWorkers = atoi(optarg);
+				errno = 0;
+				numWorkers = strtoint(optarg, &endptr, 10);
+				/*
+				 * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS
+				 * (= 64 usually) parallel jobs because that's the maximum
+				 * limit for the WaitForMultipleObjects() call.
+				 */
+				if (*endptr || errno == ERANGE || numWorkers <= 0
+#ifdef WIN32
+					|| numWorkers > MAXIMUM_WAIT_OBJECTS
+#endif
+					)
+					fatal("invalid number of parallel jobs %s", optarg);
 				break;
 
 			case 'n':			/* include schema(s) */
@@ -549,8 +562,10 @@ main(int argc, char **argv)
 				break;
 
 			case 'Z':			/* Compression Level */
-				compressLevel = atoi(optarg);
-				if (compressLevel < 0 || compressLevel > 9)
+				errno = 0;
+				compressLevel = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					compressLevel < 0 || compressLevel > 9)
 				{
 					pg_log_error("compression level must be in range 0..9");
 					exit_nicely(1);
@@ -587,8 +602,10 @@ 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)
+				errno = 0;
+				extra_float_digits = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					extra_float_digits < -15 || extra_float_digits > 3)
 				{
 					pg_log_error("extra_float_digits must be in range -15..3");
 					exit_nicely(1);
@@ -719,18 +736,6 @@ main(int argc, char **argv)
 	if (!plainText)
 		dopt.outputCreateDB = 1;
 
-	/*
-	 * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS (= 64 usually)
-	 * parallel jobs because that's the maximum limit for the
-	 * WaitForMultipleObjects() call.
-	 */
-	if (numWorkers <= 0
-#ifdef WIN32
-		|| numWorkers > MAXIMUM_WAIT_OBJECTS
-#endif
-		)
-		fatal("invalid number of parallel jobs");
-
 	/* Parallel backup only in the directory archive format so far */
 	if (archiveFormat != archDirectory && numWorkers > 1)
 		fatal("parallel backup only supported by the directory format");
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 589b4aed53..285a09aaac 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -39,6 +39,7 @@
  *-------------------------------------------------------------------------
  */
 #include "postgres_fe.h"
+#include "common/string.h"
 
 #include <ctype.h>
 #ifdef HAVE_TERMIOS_H
@@ -151,6 +152,8 @@ 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)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'a':			/* Dump data only */
@@ -181,7 +184,13 @@ main(int argc, char **argv)
 				break;
 
 			case 'j':			/* number of restore jobs */
-				numWorkers = atoi(optarg);
+				errno = 0;
+				numWorkers = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || numWorkers <= 0)
+				{
+					pg_log_error("invalid number of parallel jobs");
+					exit(1);
+				}
 				break;
 
 			case 'l':			/* Dump the TOC summary */
@@ -344,12 +353,6 @@ main(int argc, char **argv)
 		exit_nicely(1);
 	}
 
-	if (numWorkers <= 0)
-	{
-		pg_log_error("invalid number of parallel jobs");
-		exit(1);
-	}
-
 	/* See comments in pg_dump.c */
 #ifdef WIN32
 	if (numWorkers > MAXIMUM_WAIT_OBJECTS)
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 64bbda5650..f96f0d1e2a 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -104,6 +104,8 @@ 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)
 	{
+		char *endptr;
+
 		switch (option)
 		{
 			case 'b':
@@ -127,7 +129,12 @@ parseCommandLine(int argc, char *argv[])
 				break;
 
 			case 'j':
-				user_opts.jobs = atoi(optarg);
+				errno = 0;
+				user_opts.jobs = strtoint(optarg, &endptr, 10);
+				/**/
+				if (*endptr || errno == ERANGE || user_opts.jobs < 1)
+					pg_fatal("invalid number of jobs %s\n", optarg);
+					
 				break;
 
 			case 'k':
@@ -166,13 +173,17 @@ 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)
-					pg_fatal("invalid old port number\n");
+				errno = 0;
+				if ((old_cluster.port = strtoint(optarg, &endptr, 10)) <= 0 ||
+					*endptr || errno == ERANGE)
+					pg_fatal("invalid old port number %s\n", optarg);
 				break;
 
 			case 'P':
-				if ((new_cluster.port = atoi(optarg)) <= 0)
-					pg_fatal("invalid new port number\n");
+				errno = 0;
+				if ((new_cluster.port = strtoint(optarg, &endptr, 10)) <= 0 ||
+					*endptr || errno == ERANGE)
+					pg_fatal("invalid new port number %s\n", optarg);
 				break;
 
 			case 'r':
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 4aeccd93af..4020347585 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5838,6 +5838,7 @@ 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;
+		char 	   *endptr;
 
 		switch (c)
 		{
@@ -5869,8 +5870,9 @@ main(int argc, char **argv)
 				break;
 			case 'c':
 				benchmarking_option_set = true;
-				nclients = atoi(optarg);
-				if (nclients <= 0)
+				errno = 0;
+				nclients = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || nclients <= 0)
 				{
 					pg_log_fatal("invalid number of clients: \"%s\"", optarg);
 					exit(1);
@@ -5896,8 +5898,9 @@ main(int argc, char **argv)
 				break;
 			case 'j':			/* jobs */
 				benchmarking_option_set = true;
-				nthreads = atoi(optarg);
-				if (nthreads <= 0)
+				errno = 0;
+				nthreads = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || nthreads <= 0)
 				{
 					pg_log_fatal("invalid number of threads: \"%s\"", optarg);
 					exit(1);
@@ -5920,8 +5923,9 @@ main(int argc, char **argv)
 				break;
 			case 's':
 				scale_given = true;
-				scale = atoi(optarg);
-				if (scale <= 0)
+				errno = 0;
+				scale = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || scale <= 0)
 				{
 					pg_log_fatal("invalid scaling factor: \"%s\"", optarg);
 					exit(1);
@@ -5929,8 +5933,9 @@ main(int argc, char **argv)
 				break;
 			case 't':
 				benchmarking_option_set = true;
-				nxacts = atoi(optarg);
-				if (nxacts <= 0)
+				errno = 0;
+				nxacts = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || nxacts <= 0)
 				{
 					pg_log_fatal("invalid number of transactions: \"%s\"", optarg);
 					exit(1);
@@ -5938,8 +5943,9 @@ main(int argc, char **argv)
 				break;
 			case 'T':
 				benchmarking_option_set = true;
-				duration = atoi(optarg);
-				if (duration <= 0)
+				errno = 0;
+				duration = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || duration <= 0)
 				{
 					pg_log_fatal("invalid duration: \"%s\"", optarg);
 					exit(1);
@@ -6001,8 +6007,10 @@ main(int argc, char **argv)
 				break;
 			case 'F':
 				initialization_option_set = true;
-				fillfactor = atoi(optarg);
-				if (fillfactor < 10 || fillfactor > 100)
+				errno = 0;
+				fillfactor = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					fillfactor < 10 || fillfactor > 100)
 				{
 					pg_log_fatal("invalid fillfactor: \"%s\"", optarg);
 					exit(1);
@@ -6021,8 +6029,9 @@ main(int argc, char **argv)
 				break;
 			case 'P':
 				benchmarking_option_set = true;
-				progress = atoi(optarg);
-				if (progress <= 0)
+				errno = 0;
+				progress = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || progress <= 0)
 				{
 					pg_log_fatal("invalid thread progress delay: \"%s\"", optarg);
 					exit(1);
@@ -6031,11 +6040,13 @@ main(int argc, char **argv)
 			case 'R':
 				{
 					/* get a double from the beginning of option value */
-					double		throttle_value = atof(optarg);
+					double		throttle_value;
 
+					errno = 0;
+					throttle_value = strtod(optarg, &endptr);
 					benchmarking_option_set = true;
 
-					if (throttle_value <= 0.0)
+					if (*endptr || errno == ERANGE || throttle_value <= 0.0)
 					{
 						pg_log_fatal("invalid rate limit: \"%s\"", optarg);
 						exit(1);
@@ -6046,9 +6057,12 @@ main(int argc, char **argv)
 				break;
 			case 'L':
 				{
-					double		limit_ms = atof(optarg);
+					double		limit_ms;
 
-					if (limit_ms <= 0.0)
+					errno = 0;
+					limit_ms = strtod(optarg, &endptr);
+
+					if (*endptr || errno == ERANGE || limit_ms <= 0.0)
 					{
 						pg_log_fatal("invalid latency limit: \"%s\"", optarg);
 						exit(1);
@@ -6071,8 +6085,10 @@ main(int argc, char **argv)
 				break;
 			case 4:				/* sampling-rate */
 				benchmarking_option_set = true;
-				sample_rate = atof(optarg);
-				if (sample_rate <= 0.0 || sample_rate > 1.0)
+				errno = 0;
+				sample_rate = strtod(optarg, &endptr);
+				if (*endptr || errno == ERANGE ||
+					sample_rate <= 0.0 || sample_rate > 1.0)
 				{
 					pg_log_fatal("invalid sampling rate: \"%s\"", optarg);
 					exit(1);
@@ -6080,8 +6096,9 @@ main(int argc, char **argv)
 				break;
 			case 5:				/* aggregate-interval */
 				benchmarking_option_set = true;
-				agg_interval = atoi(optarg);
-				if (agg_interval <= 0)
+				errno = 0;
+				agg_interval = strtod(optarg, &endptr);
+				if (*endptr || errno == ERANGE || agg_interval <= 0)
 				{
 					pg_log_fatal("invalid number of seconds for aggregation: \"%s\"", optarg);
 					exit(1);
@@ -6117,8 +6134,9 @@ main(int argc, char **argv)
 				break;
 			case 11:			/* partitions */
 				initialization_option_set = true;
-				partitions = atoi(optarg);
-				if (partitions < 0)
+				errno = 0;
+				partitions = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || partitions < 0)
 				{
 					pg_log_fatal("invalid number of partitions: \"%s\"", optarg);
 					exit(1);
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 543401c6d6..aaed986ae1 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1039,8 +1039,11 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch,
 			}
 			if (ln)
 			{
-				lineno = atoi(ln);
-				if (lineno < 1)
+				char *endptr;
+
+				errno = 0;
+				lineno = strtoint(ln, &endptr, 10);
+				if (*endptr || errno == ERANGE || lineno < 1)
 				{
 					pg_log_error("invalid line number: %s", ln);
 					status = PSQL_CMD_ERROR;
@@ -4283,7 +4286,21 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "border") == 0)
 	{
 		if (value)
-			popt->topt.border = atoi(value);
+		{
+			char *endptr;
+			int new_value;
+
+			errno = 0;
+			new_value = strtoint(value, &endptr, 10);
+			if (*endptr || errno == ERANGE ||
+				new_value < 0 || new_value > 65535)
+			{
+				pg_log_error("\\pset: border is invalid or out of range");
+				return false;
+			}
+
+			popt->topt.border = new_value;
+		}
 	}
 
 	/* set expanded/vertical mode */
@@ -4439,7 +4456,20 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "pager_min_lines") == 0)
 	{
 		if (value)
-			popt->topt.pager_min_lines = atoi(value);
+		{
+			char *endptr;
+			int new_value;
+
+			errno = 0;
+			new_value = strtoint(value, &endptr, 10);
+			if (*endptr || errno == ERANGE || new_value < 0)
+			{
+				pg_log_error("\\pset: pager_min_lines is invalid or out of range");
+				return false;
+			}
+
+			popt->topt.pager_min_lines = new_value;
+		}
 	}
 
 	/* disable "(x rows)" footer */
@@ -4455,7 +4485,19 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "columns") == 0)
 	{
 		if (value)
-			popt->topt.columns = atoi(value);
+		{
+			char *endptr;
+			int new_value;
+
+			errno = 0;
+			new_value = strtoint(value, &endptr, 10);
+			if (*endptr || errno == ERANGE || new_value < 0)
+			{
+				pg_log_error("\\pset: column is invalid or out of range");
+				return false;
+			}
+			popt->topt.columns = new_value;
+		}
 	}
 	else
 	{
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fc0681538a..baa68d58d8 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -15,6 +15,7 @@
 #include "common.h"
 #include "common/connect.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/option_utils.h"
 #include "fe_utils/parallel_slot.h"
@@ -109,6 +110,8 @@ 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)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'h':
@@ -151,10 +154,11 @@ main(int argc, char *argv[])
 				simple_string_list_append(&indexes, optarg);
 				break;
 			case 'j':
-				concurrentCons = atoi(optarg);
-				if (concurrentCons <= 0)
+				errno = 0;
+				concurrentCons = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || concurrentCons <= 0)
 				{
-					pg_log_error("number of parallel jobs must be at least 1");
+					pg_log_error("number of parallel jobs must be at least 1: %s", optarg);
 					exit(1);
 				}
 				break;
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 61974baa78..93b563998a 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -17,6 +17,7 @@
 #include "common.h"
 #include "common/connect.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/option_utils.h"
 #include "fe_utils/parallel_slot.h"
@@ -141,6 +142,8 @@ main(int argc, char *argv[])
 
 	while ((c = getopt_long(argc, argv, "h:p:U:wWeqd:zZFat:fvj:P:", long_options, &optindex)) != -1)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'h':
@@ -192,16 +195,18 @@ main(int argc, char *argv[])
 				vacopts.verbose = true;
 				break;
 			case 'j':
-				concurrentCons = atoi(optarg);
-				if (concurrentCons <= 0)
+				errno = 0;
+				concurrentCons = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || concurrentCons <= 0)
 				{
 					pg_log_error("number of parallel jobs must be at least 1");
 					exit(1);
 				}
 				break;
 			case 'P':
-				vacopts.parallel_workers = atoi(optarg);
-				if (vacopts.parallel_workers < 0)
+				errno = 0;
+				vacopts.parallel_workers = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || vacopts.parallel_workers < 0)
 				{
 					pg_log_error("parallel workers for vacuum must be greater than or equal to zero");
 					exit(1);
@@ -220,16 +225,18 @@ main(int argc, char *argv[])
 				vacopts.skip_locked = true;
 				break;
 			case 6:
-				vacopts.min_xid_age = atoi(optarg);
-				if (vacopts.min_xid_age <= 0)
+				errno = 0;
+				vacopts.min_xid_age = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || vacopts.min_xid_age <= 0)
 				{
 					pg_log_error("minimum transaction ID age must be at least 1");
 					exit(1);
 				}
 				break;
 			case 7:
-				vacopts.min_mxid_age = atoi(optarg);
-				if (vacopts.min_mxid_age <= 0)
+				errno = 0;
+				vacopts.min_mxid_age = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE || vacopts.min_mxid_age <= 0)
 				{
 					pg_log_error("minimum multixact ID age must be at least 1");
 					exit(1);
-- 
2.27.0


----Next_Part(Fri_Jul__9_16_50_28_2021_228)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v2-0002-Make-complain-for-invalid-numeirc-values-in-envir.patch"



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH v3 1/3] Be strict in numeric parameters on command line
@ 2021-07-08 06:08 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Kyotaro Horiguchi @ 2021-07-08 06:08 UTC (permalink / raw)

Some numeric command line parameters are tolerant of valid values
followed by garbage like "123xyz".  Be strict to reject such invalid
values. Do the same for psql meta command parameters.
---
 src/bin/pg_amcheck/pg_amcheck.c        |  15 ++-
 src/bin/pg_basebackup/pg_basebackup.c  |  24 +++-
 src/bin/pg_basebackup/pg_receivewal.c  |  39 +++++--
 src/bin/pg_basebackup/pg_recvlogical.c |  44 +++++--
 src/bin/pg_checksums/pg_checksums.c    |  17 ++-
 src/bin/pg_ctl/pg_ctl.c                |  42 ++++++-
 src/bin/pg_dump/pg_dump.c              |  57 ++++++---
 src/bin/pg_dump/pg_restore.c           |  42 ++++---
 src/bin/pg_upgrade/option.c            |  30 ++++-
 src/bin/pgbench/pgbench.c              | 154 +++++++++++++++++++------
 src/bin/psql/command.c                 |  73 +++++++++++-
 src/bin/scripts/reindexdb.c            |  15 ++-
 src/bin/scripts/vacuumdb.c             |  59 ++++++++--
 13 files changed, 484 insertions(+), 127 deletions(-)

diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 4bde16fb4b..f40d58ac96 100644
--- a/src/bin/pg_amcheck/pg_amcheck.c
+++ b/src/bin/pg_amcheck/pg_amcheck.c
@@ -17,6 +17,7 @@
 #include "catalog/pg_am_d.h"
 #include "catalog/pg_namespace_d.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "common/username.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/option_utils.h"
@@ -326,10 +327,18 @@ main(int argc, char *argv[])
 				append_btree_pattern(&opts.exclude, optarg, encoding);
 				break;
 			case 'j':
-				opts.jobs = atoi(optarg);
-				if (opts.jobs < 1)
+				errno = 0;
+				opts.jobs = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("number of parallel jobs must be at least 1");
+					pg_log_error("number of parallel jobs out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || opts.jobs < 1)
+				{
+					pg_log_error("number of parallel jobs must be an integer greater than zero: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8bb0acf498..c30005f569 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2287,6 +2287,8 @@ 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)
 	{
+		char   *endptr;
+
 		switch (c)
 		{
 			case 'C':
@@ -2371,10 +2373,12 @@ main(int argc, char **argv)
 #endif
 				break;
 			case 'Z':
-				compresslevel = atoi(optarg);
-				if (compresslevel < 0 || compresslevel > 9)
+				errno = 0;
+				compresslevel = strtoint(optarg, &endptr, 10);
+				if (*endptr ||
+					errno == ERANGE || compresslevel < 0 || compresslevel > 9)
 				{
-					pg_log_error("invalid compression level \"%s\"", optarg);
+					pg_log_error("compression level must be a digit in range 0..9: \"%s\"", optarg);
 					exit(1);
 				}
 				break;
@@ -2409,10 +2413,18 @@ main(int argc, char **argv)
 				dbgetpassword = 1;
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				errno = 0;
+				standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("invalid status interval \"%s\"", optarg);
+					pg_log_error("status interval out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || standby_message_timeout < 0)
+				{
+					pg_log_error("status interval must be a non-negative integer: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index c1334fad35..fb03147fe7 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -22,6 +22,7 @@
 #include "access/xlog_internal.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "getopt_long.h"
 #include "libpq-fe.h"
 #include "receivelog.h"
@@ -520,6 +521,9 @@ 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)
 	{
+		char   *endptr;
+		int		v;
+
 		switch (c)
 		{
 			case 'D':
@@ -532,9 +536,17 @@ main(int argc, char **argv)
 				dbhost = pg_strdup(optarg);
 				break;
 			case 'p':
-				if (atoi(optarg) <= 0)
+				errno = 0;
+				v = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("invalid port number \"%s\"", optarg);
+					pg_log_error("port number out of range: \"%s\"", optarg);
+					exit(1);
+				}
+				if (*endptr || v < 1)
+				{
+					pg_log_error("port number must be an integer greater than zero: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				dbport = pg_strdup(optarg);
@@ -549,10 +561,18 @@ main(int argc, char **argv)
 				dbgetpassword = 1;
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				errno = 0;
+				standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("invalid status interval \"%s\"", optarg);
+					pg_log_error("status interval out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || standby_message_timeout < 0)
+				{
+					pg_log_error("status interval must be a non-negative integer: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
@@ -574,10 +594,13 @@ main(int argc, char **argv)
 				verbose++;
 				break;
 			case 'Z':
-				compresslevel = atoi(optarg);
-				if (compresslevel < 0 || compresslevel > 9)
+				errno = 0;
+				compresslevel = strtoint(optarg, &endptr, 10);
+				if (*endptr ||
+					errno == ERANGE || compresslevel < 0 || compresslevel > 9)
 				{
-					pg_log_error("invalid compression level \"%s\"", optarg);
+					pg_log_error("compression level must be a digit in range 0..9: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 76bd153fac..9bc4902033 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -23,6 +23,7 @@
 #include "common/fe_memutils.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "getopt_long.h"
 #include "libpq-fe.h"
 #include "libpq/pqsignal.h"
@@ -732,6 +733,9 @@ main(int argc, char **argv)
 	while ((c = getopt_long(argc, argv, "E:f:F:nvtd:h:p:U:wWI:o:P:s:S:",
 							long_options, &option_index)) != -1)
 	{
+		char   *endptr;
+		int		v;
+
 		switch (c)
 		{
 /* general options */
@@ -739,10 +743,18 @@ main(int argc, char **argv)
 				outfile = pg_strdup(optarg);
 				break;
 			case 'F':
-				fsync_interval = atoi(optarg) * 1000;
-				if (fsync_interval < 0)
+				errno = 0;
+				fsync_interval = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("invalid fsync interval \"%s\"", optarg);
+					pg_log_error("fsync interval out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || fsync_interval < 0)
+				{
+					pg_log_error("fsync interval must be a non-negative integer: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
@@ -763,9 +775,17 @@ main(int argc, char **argv)
 				dbhost = pg_strdup(optarg);
 				break;
 			case 'p':
-				if (atoi(optarg) <= 0)
+				errno = 0;
+				v = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("invalid port number \"%s\"", optarg);
+					pg_log_error("port number out of range: \"%s\"", optarg);
+					exit(1);
+				}
+				if (*endptr || v < 1)
+				{
+					pg_log_error("port number must be an integer greater than zero: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				dbport = pg_strdup(optarg);
@@ -820,10 +840,18 @@ main(int argc, char **argv)
 				plugin = pg_strdup(optarg);
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				errno = 0;
+				standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("invalid status interval \"%s\"", optarg);
+					pg_log_error("status interval out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || standby_message_timeout < 0)
+				{
+					pg_log_error("status interval must be a non-negative integer: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 3c326906e2..78a1d4ef38 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -24,6 +24,7 @@
 #include "common/file_perm.h"
 #include "common/file_utils.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "getopt_long.h"
 #include "pg_getopt.h"
 #include "storage/bufpage.h"
@@ -506,6 +507,9 @@ main(int argc, char *argv[])
 
 	while ((c = getopt_long(argc, argv, "cD:deNPf:v", long_options, &option_index)) != -1)
 	{
+		char   *endptr;
+		int		v;
+
 		switch (c)
 		{
 			case 'c':
@@ -518,9 +522,18 @@ main(int argc, char *argv[])
 				mode = PG_MODE_ENABLE;
 				break;
 			case 'f':
-				if (atoi(optarg) == 0)
+				errno = 0;
+				v = strtoint(optarg, &endptr, 10);
+				if(*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("invalid filenode specification, must be numeric: %s", optarg);
+					pg_log_error("filenode specification out of range: %s",
+								 optarg);
+					exit(1);
+				}
+				if(*endptr || v < 1)
+				{
+					pg_log_error("filenode specification must be an integer greater than zero: %s",
+								 optarg);
 					exit(1);
 				}
 				only_filenode = pstrdup(optarg);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7985da0a94..0f72ef016b 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -76,6 +76,7 @@ typedef enum
 
 #define WAITS_PER_SEC	10		/* should divide USEC_PER_SEC evenly */
 
+static bool do_wait_arg = false;
 static bool do_wait = true;
 static int	wait_seconds = DEFAULT_WAIT;
 static bool wait_seconds_arg = false;
@@ -2331,6 +2332,8 @@ main(int argc, char **argv)
 	/* process command-line options */
 	while (optind < argc)
 	{
+		char *endptr;
+
 		while ((c = getopt_long(argc, argv, "cD:e:l:m:N:o:p:P:sS:t:U:wW",
 								long_options, &option_index)) != -1)
 		{
@@ -2396,7 +2399,20 @@ main(int argc, char **argv)
 #endif
 					break;
 				case 't':
-					wait_seconds = atoi(optarg);
+					errno = 0;
+					wait_seconds = strtoint(optarg, &endptr, 10);
+					if (*endptr == 0 && errno == ERANGE)
+					{
+						pg_log_error("timeout value out of range: \"%s\"",
+									 optarg);
+						exit(1);
+					}
+					if (*endptr || wait_seconds < 0)
+					{
+						pg_log_error("timeout value must be a non-negative integer: \"%s\"",
+									 optarg);
+						exit(1);
+					}
 					wait_seconds_arg = true;
 					break;
 				case 'U':
@@ -2408,6 +2424,7 @@ main(int argc, char **argv)
 					break;
 				case 'w':
 					do_wait = true;
+					do_wait_arg = true;
 					break;
 				case 'W':
 					do_wait = false;
@@ -2459,7 +2476,20 @@ main(int argc, char **argv)
 				}
 				ctl_command = KILL_COMMAND;
 				set_sig(argv[++optind]);
-				killproc = atol(argv[++optind]);
+				errno = 0;
+				killproc = strtol(argv[++optind], &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
+				{
+					pg_log_error("process ID out of range: \"%s\"",
+								 argv[optind]);
+					exit(1);
+				}
+				if (*endptr || killproc < 0)
+				{
+					pg_log_error("process ID must be a non-negative integer: \"%s\"",
+								 argv[optind]);
+					exit(1);
+				}
 			}
 #ifdef WIN32
 			else if (strcmp(argv[optind], "register") == 0)
@@ -2514,6 +2544,14 @@ main(int argc, char **argv)
 		do_wait = false;
 	}
 
+	if (wait_seconds == 0 && do_wait)
+	{
+		/* Warn if user instructed to wait but we actually don't */
+		if (!silent_mode && do_wait_arg)
+			write_stderr(_("%s: WARNING: -w is ignored because timeout is set to 0\n"), progname);
+		do_wait = false;
+	}
+
 	if (pg_data)
 	{
 		snprintf(postopts_file, MAXPGPATH, "%s/postmaster.opts", pg_data);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 321152151d..8ef29b37f6 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -54,6 +54,7 @@
 #include "catalog/pg_trigger_d.h"
 #include "catalog/pg_type_d.h"
 #include "common/connect.h"
+#include "common/string.h"
 #include "dumputils.h"
 #include "fe_utils/string_utils.h"
 #include "getopt_long.h"
@@ -103,6 +104,17 @@ static Oid	g_last_builtin_oid; /* value of the last builtin oid */
 /* The specified names/patterns should to match at least one entity */
 static int	strict_names = 0;
 
+/*
+ * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS (= 64 usually)
+ * parallel jobs because that's the maximum limit for the
+ * WaitForMultipleObjects() call.
+ */
+#ifndef WIN32
+#define	MAX_NUM_WORKERS INT_MAX
+#else
+#define	MAX_NUM_WORKERS MAXIMUM_WAIT_OBJECTS
+#endif
+
 /*
  * Object inclusion/exclusion lists
  *
@@ -486,7 +498,21 @@ main(int argc, char **argv)
 				break;
 
 			case 'j':			/* number of dump jobs */
-				numWorkers = atoi(optarg);
+				errno = 0;
+				numWorkers = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 &&
+					(errno == ERANGE || numWorkers > MAX_NUM_WORKERS))
+				{
+					pg_log_error("number of parallel jobs out of range: \"%s\"",
+								 optarg);
+					exit_nicely(1);
+				}
+				if (*endptr || numWorkers <= 0)
+				{
+					pg_log_error("number of parallel jobs must be an integer greater than zero: \"%s\"",
+								 optarg);
+					exit_nicely(1);
+				}
 				break;
 
 			case 'n':			/* include schema(s) */
@@ -549,10 +575,12 @@ main(int argc, char **argv)
 				break;
 
 			case 'Z':			/* Compression Level */
-				compressLevel = atoi(optarg);
-				if (compressLevel < 0 || compressLevel > 9)
+				errno = 0;
+				compressLevel = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					compressLevel < 0 || compressLevel > 9)
 				{
-					pg_log_error("compression level must be in range 0..9");
+					pg_log_error("compression level must be a digit in range 0..9: \"%s\"", optarg);
 					exit_nicely(1);
 				}
 				break;
@@ -587,10 +615,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)
+				errno = 0;
+				extra_float_digits = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					extra_float_digits < -15 || extra_float_digits > 3)
 				{
-					pg_log_error("extra_float_digits must be in range -15..3");
+					pg_log_error("extra_float_digits must be an integer in range -15..3: \"%s\"",
+						optarg);
 					exit_nicely(1);
 				}
 				break;
@@ -719,18 +750,6 @@ main(int argc, char **argv)
 	if (!plainText)
 		dopt.outputCreateDB = 1;
 
-	/*
-	 * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS (= 64 usually)
-	 * parallel jobs because that's the maximum limit for the
-	 * WaitForMultipleObjects() call.
-	 */
-	if (numWorkers <= 0
-#ifdef WIN32
-		|| numWorkers > MAXIMUM_WAIT_OBJECTS
-#endif
-		)
-		fatal("invalid number of parallel jobs");
-
 	/* Parallel backup only in the directory archive format so far */
 	if (archiveFormat != archDirectory && numWorkers > 1)
 		fatal("parallel backup only supported by the directory format");
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 589b4aed53..3bb5a48c55 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -39,6 +39,7 @@
  *-------------------------------------------------------------------------
  */
 #include "postgres_fe.h"
+#include "common/string.h"
 
 #include <ctype.h>
 #ifdef HAVE_TERMIOS_H
@@ -52,6 +53,13 @@
 
 static void usage(const char *progname);
 
+/* See comments in pg_dump.c */
+#ifndef WIN32
+#define MAX_NUM_WORKERS INT_MAX
+#else
+#define MAX_NUM_WORKERS MAXIMUM_WAIT_OBJECTS
+#endif
+
 int
 main(int argc, char **argv)
 {
@@ -151,6 +159,8 @@ 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)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'a':			/* Dump data only */
@@ -181,7 +191,21 @@ main(int argc, char **argv)
 				break;
 
 			case 'j':			/* number of restore jobs */
-				numWorkers = atoi(optarg);
+				errno = 0;
+				numWorkers = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 &&
+					(errno == ERANGE || numWorkers > MAX_NUM_WORKERS))
+				{
+					pg_log_error("number of parallel jobs out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || numWorkers <= 0)
+				{
+					pg_log_error("number of parallel jobs must be an integer greater than zero: \"%s\"",
+								 optarg);
+					exit(1);
+				}
 				break;
 
 			case 'l':			/* Dump the TOC summary */
@@ -344,22 +368,6 @@ main(int argc, char **argv)
 		exit_nicely(1);
 	}
 
-	if (numWorkers <= 0)
-	{
-		pg_log_error("invalid number of parallel jobs");
-		exit(1);
-	}
-
-	/* See comments in pg_dump.c */
-#ifdef WIN32
-	if (numWorkers > MAXIMUM_WAIT_OBJECTS)
-	{
-		pg_log_error("maximum number of parallel jobs is %d",
-					 MAXIMUM_WAIT_OBJECTS);
-		exit(1);
-	}
-#endif
-
 	/* Can't do single-txn mode with multiple connections */
 	if (opts->single_txn && numWorkers > 1)
 	{
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 64bbda5650..c014bbca0d 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -104,6 +104,8 @@ 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)
 	{
+		char   *endptr;
+
 		switch (option)
 		{
 			case 'b':
@@ -127,7 +129,15 @@ parseCommandLine(int argc, char *argv[])
 				break;
 
 			case 'j':
-				user_opts.jobs = atoi(optarg);
+				errno = 0;
+				user_opts.jobs = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
+					pg_fatal("number of parallel jobs out of range: \"%s\"\n",
+							 optarg);
+				if (*endptr || user_opts.jobs < 1)
+					pg_fatal("number of parallel jobs must be an integer greater than zero: \"%s\"\n",
+							 optarg);
+					
 				break;
 
 			case 'k':
@@ -166,13 +176,23 @@ 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)
-					pg_fatal("invalid old port number\n");
+				errno = 0;
+				old_cluster.port = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
+					pg_fatal("old port number out of range: \"%s\"\n", optarg);
+				if (*endptr || old_cluster.port <= 0)
+					pg_fatal("old port number must be an integer greater than zero: \"%s\"\n",
+							 optarg);
 				break;
 
 			case 'P':
-				if ((new_cluster.port = atoi(optarg)) <= 0)
-					pg_fatal("invalid new port number\n");
+				errno = 0;
+				new_cluster.port = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
+					pg_fatal("new port number out of range: \"%s\"\n", optarg);
+				if (*endptr || new_cluster.port <= 0)
+					pg_fatal("new port number must be an integer greater than zero: \"%s\"\n",
+							 optarg);
 				break;
 
 			case 'r':
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 364b5a2e47..1c3b5836c1 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5856,6 +5856,7 @@ 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;
+		char 	   *endptr;
 
 		switch (c)
 		{
@@ -5887,10 +5888,18 @@ main(int argc, char **argv)
 				break;
 			case 'c':
 				benchmarking_option_set = true;
-				nclients = atoi(optarg);
-				if (nclients <= 0)
+				errno = 0;
+				nclients = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_fatal("invalid number of clients: \"%s\"", optarg);
+					pg_log_fatal("number of clients out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || nclients <= 0)
+				{
+					pg_log_fatal("number of clients must be an integer greater than zero: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 #ifdef HAVE_GETRLIMIT
@@ -5914,10 +5923,18 @@ main(int argc, char **argv)
 				break;
 			case 'j':			/* jobs */
 				benchmarking_option_set = true;
-				nthreads = atoi(optarg);
-				if (nthreads <= 0)
+				errno = 0;
+				nthreads = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_fatal("invalid number of threads: \"%s\"", optarg);
+					pg_log_fatal("number of threads out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || nthreads <= 0)
+				{
+					pg_log_fatal("number of threads must be an integer greater than zero: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 #ifndef ENABLE_THREAD_SAFETY
@@ -5938,28 +5955,50 @@ main(int argc, char **argv)
 				break;
 			case 's':
 				scale_given = true;
-				scale = atoi(optarg);
-				if (scale <= 0)
+				errno = 0;
+				scale = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_fatal("invalid scaling factor: \"%s\"", optarg);
+					pg_log_fatal("scaling factor out of range: \"%s\"", optarg);
+					exit(1);
+				}
+				if (*endptr || scale <= 0)
+				{
+					pg_log_fatal("scaling factor must be an integer greater than zero: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
 			case 't':
 				benchmarking_option_set = true;
-				nxacts = atoi(optarg);
-				if (nxacts <= 0)
+				errno = 0;
+				nxacts = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_fatal("invalid number of transactions: \"%s\"", optarg);
+					pg_log_fatal("number of transactions out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || nxacts <= 0)
+				{
+					pg_log_fatal("number of transactions must be an integer greater than zero: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
 			case 'T':
 				benchmarking_option_set = true;
-				duration = atoi(optarg);
-				if (duration <= 0)
+				errno = 0;
+				duration = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_fatal("invalid duration: \"%s\"", optarg);
+					pg_log_fatal("duration out of range: \"%s\"", optarg);
+					exit(1);
+				}
+				if (*endptr || duration <= 0)
+				{
+					pg_log_fatal("duration must be an integer greater than zero: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
@@ -6019,10 +6058,13 @@ main(int argc, char **argv)
 				break;
 			case 'F':
 				initialization_option_set = true;
-				fillfactor = atoi(optarg);
-				if (fillfactor < 10 || fillfactor > 100)
+				errno = 0;
+				fillfactor = strtoint(optarg, &endptr, 10);
+				if (*endptr || errno == ERANGE ||
+					fillfactor < 10 || fillfactor > 100)
 				{
-					pg_log_fatal("invalid fillfactor: \"%s\"", optarg);
+					pg_log_fatal("fillfactor must be an ineger between 10 and 100: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
@@ -6039,23 +6081,38 @@ main(int argc, char **argv)
 				break;
 			case 'P':
 				benchmarking_option_set = true;
-				progress = atoi(optarg);
-				if (progress <= 0)
+				errno = 0;
+				progress = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_fatal("invalid thread progress delay: \"%s\"", optarg);
+					pg_log_fatal("thread progress delay out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || progress <= 0)
+				{
+					pg_log_fatal("thread progress delay must be an integer greater than zero: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
 			case 'R':
 				{
 					/* get a double from the beginning of option value */
-					double		throttle_value = atof(optarg);
+					double		throttle_value;
 
+					errno = 0;
+					throttle_value = strtod(optarg, &endptr);
 					benchmarking_option_set = true;
 
-					if (throttle_value <= 0.0)
+					if (*endptr == 0 && errno == ERANGE)
 					{
-						pg_log_fatal("invalid rate limit: \"%s\"", optarg);
+						pg_log_fatal("rate limit out of range: \"%s\"", optarg);
+						exit(1);
+					}
+					if (*endptr || throttle_value <= 0.0)
+					{
+						pg_log_fatal("rate limit must be a real number greater than zero: \"%s\"", optarg);
 						exit(1);
 					}
 					/* Invert rate limit into per-transaction delay in usec */
@@ -6064,11 +6121,20 @@ main(int argc, char **argv)
 				break;
 			case 'L':
 				{
-					double		limit_ms = atof(optarg);
+					double		limit_ms;
 
-					if (limit_ms <= 0.0)
+					errno = 0;
+					limit_ms = strtod(optarg, &endptr);
+
+					if (*endptr == 0 && errno == ERANGE)
+					{
+						pg_log_fatal("latency limit out of range: \"%s\"",
+									 optarg);
+						exit(1);
+					}
+					if (*endptr || limit_ms <= 0.0)
 					{
-						pg_log_fatal("invalid latency limit: \"%s\"", optarg);
+						pg_log_fatal("latency limit must be a real number greater than zero: \"%s\"", optarg);
 						exit(1);
 					}
 					benchmarking_option_set = true;
@@ -6089,19 +6155,27 @@ main(int argc, char **argv)
 				break;
 			case 4:				/* sampling-rate */
 				benchmarking_option_set = true;
-				sample_rate = atof(optarg);
-				if (sample_rate <= 0.0 || sample_rate > 1.0)
+				errno = 0;
+				sample_rate = strtod(optarg, &endptr);
+				if (*endptr || errno == ERANGE ||
+					sample_rate <= 0.0 || sample_rate > 1.0)
 				{
-					pg_log_fatal("invalid sampling rate: \"%s\"", optarg);
+					pg_log_fatal("sampling rate must be an real number between 0.0 and 1.0: \"%s\"", optarg);
 					exit(1);
 				}
 				break;
 			case 5:				/* aggregate-interval */
 				benchmarking_option_set = true;
-				agg_interval = atoi(optarg);
-				if (agg_interval <= 0)
+				errno = 0;
+				agg_interval = strtod(optarg, &endptr);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_fatal("invalid number of seconds for aggregation: \"%s\"", optarg);
+					pg_log_fatal("aggregate interval out of range: \"%s\"", optarg);
+					exit(1);
+				}
+				if (*endptr || agg_interval <= 0)
+				{
+					pg_log_fatal("aggregate interval must be a real number greater than zero: \"%s\"", optarg);
 					exit(1);
 				}
 				break;
@@ -6135,10 +6209,18 @@ main(int argc, char **argv)
 				break;
 			case 11:			/* partitions */
 				initialization_option_set = true;
-				partitions = atoi(optarg);
-				if (partitions < 0)
+				errno = 0;
+				partitions = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_fatal("invalid number of partitions: \"%s\"", optarg);
+					pg_log_fatal("number of partitions out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || partitions < 0)
+				{
+					pg_log_fatal("number of partitions must be a non-negative integer: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index d704c4220c..13074051b3 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1040,10 +1040,18 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch,
 			}
 			if (ln)
 			{
-				lineno = atoi(ln);
-				if (lineno < 1)
+				char *endptr;
+
+				errno = 0;
+				lineno = strtoint(ln, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("invalid line number: %s", ln);
+					pg_log_error("line number out of range: %s", ln);
+					status = PSQL_CMD_ERROR;
+				}
+				if (*endptr || lineno < 1)
+				{
+					pg_log_error("line number must be an integer greater than zero: %s", ln);
 					status = PSQL_CMD_ERROR;
 				}
 			}
@@ -4284,7 +4292,25 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "border") == 0)
 	{
 		if (value)
-			popt->topt.border = atoi(value);
+		{
+			char *endptr;
+			int new_value;
+
+			errno = 0;
+			new_value = strtoint(value, &endptr, 10);
+			if (*endptr == 0 && (errno == ERANGE || new_value > 65535))
+			{
+				pg_log_error("\\pset: border out of range");
+				return false;
+			}
+			if (*endptr || new_value < 0)
+			{
+				pg_log_error("\\pset: border must be an integer greater than zero");
+				return false;
+			}
+
+			popt->topt.border = new_value;
+		}
 	}
 
 	/* set expanded/vertical mode */
@@ -4440,7 +4466,25 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "pager_min_lines") == 0)
 	{
 		if (value)
-			popt->topt.pager_min_lines = atoi(value);
+		{
+			char *endptr;
+			int new_value;
+
+			errno = 0;
+			new_value = strtoint(value, &endptr, 10);
+			if (*endptr == 0 && errno == ERANGE)
+			{
+				pg_log_error("\\pset: pager_min_lines out of range");
+				return false;
+			}
+			if (*endptr || new_value < 0)
+			{
+				pg_log_error("\\pset: pager_min_lines must be a non-negative integer");
+				return false;
+			}
+
+			popt->topt.pager_min_lines = new_value;
+		}
 	}
 
 	/* disable "(x rows)" footer */
@@ -4456,7 +4500,24 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "columns") == 0)
 	{
 		if (value)
-			popt->topt.columns = atoi(value);
+		{
+			char *endptr;
+			int new_value;
+
+			errno = 0;
+			new_value = strtoint(value, &endptr, 10);
+			if (*endptr == 0 && errno == ERANGE)
+			{
+				pg_log_error("\\pset: column out of range");
+				return false;
+			}
+			if (*endptr || new_value < 0)
+			{
+				pg_log_error("\\pset: column must be a non-negative integer");
+				return false;
+			}
+			popt->topt.columns = new_value;
+		}
 	}
 	else
 	{
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fc0681538a..42d4d20768 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -15,6 +15,7 @@
 #include "common.h"
 #include "common/connect.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/option_utils.h"
 #include "fe_utils/parallel_slot.h"
@@ -109,6 +110,8 @@ 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)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'h':
@@ -151,10 +154,16 @@ main(int argc, char *argv[])
 				simple_string_list_append(&indexes, optarg);
 				break;
 			case 'j':
-				concurrentCons = atoi(optarg);
-				if (concurrentCons <= 0)
+				errno = 0;
+				concurrentCons = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("number of parallel jobs must be at least 1");
+					pg_log_error("number of parallel jobs out of range: %s", optarg);
+					exit(1);
+				}
+				if (*endptr || concurrentCons <= 0)
+				{
+					pg_log_error("number of parallel jobs must be an integer greater than zero: %s", optarg);
 					exit(1);
 				}
 				break;
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 61974baa78..6b2a34edd0 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -17,6 +17,7 @@
 #include "common.h"
 #include "common/connect.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "fe_utils/cancel.h"
 #include "fe_utils/option_utils.h"
 #include "fe_utils/parallel_slot.h"
@@ -141,6 +142,8 @@ main(int argc, char *argv[])
 
 	while ((c = getopt_long(argc, argv, "h:p:U:wWeqd:zZFat:fvj:P:", long_options, &optindex)) != -1)
 	{
+		char *endptr;
+
 		switch (c)
 		{
 			case 'h':
@@ -192,18 +195,34 @@ main(int argc, char *argv[])
 				vacopts.verbose = true;
 				break;
 			case 'j':
-				concurrentCons = atoi(optarg);
-				if (concurrentCons <= 0)
+				errno = 0;
+				concurrentCons = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("number of parallel jobs must be at least 1");
+					pg_log_error("number of parallel jobs out of range: \"%s\"",
+						optarg);
+					exit(1);
+				}
+				if (*endptr || concurrentCons <= 0)
+				{
+					pg_log_error("number of parallel jobs must be an integer greater than zero: \"%s\"",
+								 optarg);
 					exit(1);
 				}
 				break;
 			case 'P':
-				vacopts.parallel_workers = atoi(optarg);
-				if (vacopts.parallel_workers < 0)
+				errno = 0;
+				vacopts.parallel_workers = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("parallel workers for vacuum must be greater than or equal to zero");
+					pg_log_error("parallel workers out of range: \"%s\"",
+								 optarg);
+					exit(1);
+				}
+				if (*endptr || vacopts.parallel_workers < 0)
+				{
+					pg_log_error("parallel workers for vacuum must be a non-negative integer: \"%s\"",
+						optarg);
 					exit(1);
 				}
 				break;
@@ -220,18 +239,34 @@ main(int argc, char *argv[])
 				vacopts.skip_locked = true;
 				break;
 			case 6:
-				vacopts.min_xid_age = atoi(optarg);
-				if (vacopts.min_xid_age <= 0)
+				errno = 0;
+				vacopts.min_xid_age = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("minimum transaction ID age must be at least 1");
+					pg_log_error("minimum transaction ID age out of range: \"%s\"",
+						optarg);
+					exit(1);
+				}
+				if (*endptr || vacopts.min_xid_age <= 0)
+				{
+					pg_log_error("minimum transaction ID age must be an integer greater than zero: \"%s\"",
+						optarg);
 					exit(1);
 				}
 				break;
 			case 7:
-				vacopts.min_mxid_age = atoi(optarg);
-				if (vacopts.min_mxid_age <= 0)
+				errno = 0;
+				vacopts.min_mxid_age = strtoint(optarg, &endptr, 10);
+				if (*endptr == 0 && errno == ERANGE)
 				{
-					pg_log_error("minimum multixact ID age must be at least 1");
+					pg_log_error("minimum multixact ID age out of range: \"%s\"",
+						optarg);
+					exit(1);
+				}
+				if (*endptr || vacopts.min_mxid_age <= 0)
+				{
+					pg_log_error("minimum multixact ID age must be an integer greater than zero: \"%s\"",
+						optarg);
 					exit(1);
 				}
 				break;
-- 
2.27.0


----Next_Part(Wed_Jul_14_10_35_56_2021_265)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v3-0002-Make-complain-for-invalid-numeirc-values-in-envir.patch"



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
@ 2025-06-16 12:47 Jelte Fennema-Nio <[email protected]>
  2025-06-17 04:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Ashutosh Bapat <[email protected]>
  2025-06-18 03:58 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
  2025-06-20 13:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
  2025-06-22 20:50 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-25 00:01 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
  0 siblings, 5 replies; 27+ messages in thread

From: Jelte Fennema-Nio @ 2025-06-16 12:47 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>; +Cc: David G. Johnston <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

The new PG19 development cycle is starting soon. So that seemed like a
good excuse to make some big improvements to the commitfest app. My
plan is to deploy these changes on the 30th of June. So that we can
start the new cycle fresh with these changes. As always feedback on
these changes is very welcome. The below will contain some links to
the staging environment both username and password for the http auth
are "pgtest".


# Introducing tags

This has been requested by many people. New tags can currently only be
created through the admin interface, but they can be assigned to a
patch by anyone. There are a bunch of tags that I added by default.
You can also easily filter patches by a specific tag by clicking on a
tag on a commitfest overview page. See the tags.png screenshot, or the
staging environment:

https://commitfest-test.postgresql.org/53/

Big thanks to Jacob for the initial work on this feature.

# Draft CF

There's now an additional Draft CF. People can move patches there as a
way of not forgetting about them. CFBot will rerun these patches less
frequently (exact behaviour TBD). Draft CFs are never "In Progress"
and are open until the final CF of the release cycle becomes "In
Progress". So PG19-Drafts will close on February 28th 2026, and at the
same moment PG20-Drafts will be opened.

Big thanks to David G Johnston for the initial work on this feature,
and early review/feedback on all my changes to it.

# Automated commitfest open/close/create

Right now people manually have to open/close/create commitfests at our
regular cadence. This now actually encodes this cadence in the app
itself. A new open commitfest is automatically created when needed. As
well as commitfests automatically getting started or closed. The only
commitfest that's not fully automated is the last one of the cycle,
since that ends when the feature freeze starts (and the feature freeze
isn't always the same date). To handle that the RMT (or anyone with
access) should change the enddate of the final commitfest in the admin
interface to the new date. That will need to happen before March 31st.

A *bikesheddable* change here is the names that the newly created
commitfests get automatically. Given we now have a Drafts commitfest,
it seemed reasonable to align the naming of the regular commitfests
with that. The draft commitfest will be called:

- PG19-Drafts

And our already well-known commitfests for PG19 will be called as follows:

- PG19-1 (previously 2025-07)
- PG19-2 (previously 2025-09)
- PG19-3 (previously 2025-11)
- PG19-4 (previously 2026-01)
- PG19-Final (previously 2026-03)

The dates will be the same, the name will simply not be of the
{year}-{month} format anymore. The actual dates will still be easily
visible though. So the intent is that no-one loses information, but
instead people will gain information because it's clear what Postgres
version a commitfest is about.


# New homepage

The homepage is revamped. It now shows only "In Progress", "Open",
"Draft" and "Previous" commitfests and it also shows names & dates for
"Next open CF" and "Final CF of the release cycle". See homepage.png
screenshot for details or go to
https://commitfest-test.postgresql.org/

The full list of commitfests is still available at:
https://commitfest-test.postgresql.org/commitfest_history/ This page
is linked to at the bottom most link on the homepage.


# Help Page

There's now a help page for new users which explains how this app is
used and what certain terminology means:
https://commitfest-test.postgresql.org/help/

# Small fixes

Flonents Tselai made wiki and git links display correctly.


Attachments:

  [image/png] tags.png (524.3K, ../../CAGECzQQ4LyfZi+WYugAMn57Q20u-++4wbvk4f-RA2iic9VMNjQ@mail.gmail.com/2-tags.png)
  download | view image

  [image/png] homepage.png (247.4K, ../../CAGECzQQ4LyfZi+WYugAMn57Q20u-++4wbvk4f-RA2iic9VMNjQ@mail.gmail.com/3-homepage.png)
  download | view image

^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
@ 2025-06-17 04:21 ` Ashutosh Bapat <[email protected]>
  2025-06-17 04:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tom Lane <[email protected]>
  2025-06-17 08:05   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  4 siblings, 2 replies; 27+ messages in thread

From: Ashutosh Bapat @ 2025-06-17 04:21 UTC (permalink / raw)
  To: Jelte Fennema-Nio <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David G. Johnston <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On Mon, Jun 16, 2025 at 6:48 PM Jelte Fennema-Nio <[email protected]> wrote:
>
> The new PG19 development cycle is starting soon. So that seemed like a
> good excuse to make some big improvements to the commitfest app. My
> plan is to deploy these changes on the 30th of June. So that we can
> start the new cycle fresh with these changes. As always feedback on
> these changes is very welcome. The below will contain some links to
> the staging environment both username and password for the http auth
> are "pgtest".
>

Huge thanks for the huge changes.

Coinciding tooling changes with a milestone in tool usage has
potential to disrupt both. But in this case, the things seem to be
working in the staging environment. So I hope the start of PG19-1
commitfest will be smoother and a better experience.

Some comments below.

>
> # Draft CF
>
> There's now an additional Draft CF. People can move patches there as a
> way of not forgetting about them. CFBot will rerun these patches less
> frequently (exact behaviour TBD). Draft CFs are never "In Progress"
> and are open until the final CF of the release cycle becomes "In
> Progress". So PG19-Drafts will close on February 28th 2026, and at the
> same moment PG20-Drafts will be opened.
>
> Big thanks to David G Johnston for the initial work on this feature,
> and early review/feedback on all my changes to it.

This is interesting. I liked the idea of making Draft CF link part of
the landing page. People can find draft patches easily in case they
want to try those out and labelling them as "Draft" will set the
expectations right.

>
> # Automated commitfest open/close/create
>
> Right now people manually have to open/close/create commitfests at our
> regular cadence. This now actually encodes this cadence in the app
> itself. A new open commitfest is automatically created when needed. As
> well as commitfests automatically getting started or closed. The only
> commitfest that's not fully automated is the last one of the cycle,
> since that ends when the feature freeze starts (and the feature freeze
> isn't always the same date). To handle that the RMT (or anyone with
> access) should change the enddate of the final commitfest in the admin
> interface to the new date. That will need to happen before March 31st.
>
> A *bikesheddable* change here is the names that the newly created
> commitfests get automatically. Given we now have a Drafts commitfest,
> it seemed reasonable to align the naming of the regular commitfests
> with that. The draft commitfest will be called:
>
> - PG19-Drafts
>
> And our already well-known commitfests for PG19 will be called as follows:
>
> - PG19-1 (previously 2025-07)
> - PG19-2 (previously 2025-09)
> - PG19-3 (previously 2025-11)
> - PG19-4 (previously 2026-01)
> - PG19-Final (previously 2026-03)
>
> The dates will be the same, the name will simply not be of the
> {year}-{month} format anymore. The actual dates will still be easily
> visible though. So the intent is that no-one loses information, but
> instead people will gain information because it's clear what Postgres
> version a commitfest is about.
>
>
> # New homepage
>
> The homepage is revamped. It now shows only "In Progress", "Open",
> "Draft" and "Previous" commitfests and it also shows names & dates for
> "Next open CF" and "Final CF of the release cycle". See homepage.png
> screenshot for details or go to
> https://commitfest-test.postgresql.org/

I like this as well. I feel "In Progress" doesn't convey that the
column contains the dates when the CFs are active. It can be easily
confused with "In progress" CF. How about just "Dates" or "Duration"?

>
> The full list of commitfests is still available at:
> https://commitfest-test.postgresql.org/commitfest_history/ This page
> is linked to at the bottom most link on the homepage.
>

All patches in the current commitfest
All patches in the open commitfest

Are those two needed anymore since the CF links are already available?

Create a new commitfest entry - if we are creating the CFs
automatically, is this link that useful? And it could be a button/link
in the Commitfest section itself just before or after the table of
relevant commitfests.

-- 
Best Wishes,
Ashutosh Bapat





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-17 04:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Ashutosh Bapat <[email protected]>
@ 2025-06-17 04:41   ` Tom Lane <[email protected]>
  2025-06-17 07:05     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  1 sibling, 1 reply; 27+ messages in thread

From: Tom Lane @ 2025-06-17 04:41 UTC (permalink / raw)
  To: Ashutosh Bapat <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; PostgreSQL Hackers <[email protected]>; David G. Johnston <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

Ashutosh Bapat <[email protected]> writes:
> On Mon, Jun 16, 2025 at 6:48 PM Jelte Fennema-Nio <[email protected]> wrote:
>> The new PG19 development cycle is starting soon. So that seemed like a
>> good excuse to make some big improvements to the commitfest app. My
>> plan is to deploy these changes on the 30th of June.

> Coinciding tooling changes with a milestone in tool usage has
> potential to disrupt both.

Yeah.  I think it might be smarter to push these changes a bit earlier
than the 30th, maybe by a week?  Better to file down any rough edges
before we start the new CF.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-17 04:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Ashutosh Bapat <[email protected]>
  2025-06-17 04:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tom Lane <[email protected]>
@ 2025-06-17 07:05     ` Jelte Fennema-Nio <[email protected]>
  2025-06-23 01:37       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tatsuo Ishii <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Jelte Fennema-Nio @ 2025-06-17 07:05 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Ashutosh Bapat <[email protected]>; PostgreSQL Hackers <[email protected]>; David G. Johnston <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On Tue, 17 Jun 2025 at 06:41, Tom Lane <[email protected]> wrote:
> Yeah.  I think it might be smarter to push these changes a bit earlier
> than the 30th, maybe by a week?  Better to file down any rough edges
> before we start the new CF.

Sounds good to me. Unless there are big objections, I'll deploy this
on the 23rd.





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-17 04:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Ashutosh Bapat <[email protected]>
  2025-06-17 04:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tom Lane <[email protected]>
  2025-06-17 07:05     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
@ 2025-06-23 01:37       ` Tatsuo Ishii <[email protected]>
  2025-06-23 06:46         ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Tatsuo Ishii @ 2025-06-23 01:37 UTC (permalink / raw)
  To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]

> Sounds good to me. Unless there are big objections, I'll deploy this
> on the 23rd.

Sorry if this has been already reported or fixed. I tried "Personal
Dashboard".

https://commitfest.postgresql.org/me/

And I found "Author" column is shown as "+4207-35" which does not seem
to be an author name. Likewise followings columns seem to show
inappropriate contents.

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-17 04:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Ashutosh Bapat <[email protected]>
  2025-06-17 04:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tom Lane <[email protected]>
  2025-06-17 07:05     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-23 01:37       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tatsuo Ishii <[email protected]>
@ 2025-06-23 06:46         ` Jelte Fennema-Nio <[email protected]>
  2025-06-23 06:52           ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tatsuo Ishii <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Jelte Fennema-Nio @ 2025-06-23 06:46 UTC (permalink / raw)
  To: Tatsuo Ishii <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]

On Mon, 23 Jun 2025 at 03:37, Tatsuo Ishii <[email protected]> wrote:
> And I found "Author" column is shown as "+4207-35" which does not seem
> to be an author name. Likewise followings columns seem to show
> inappropriate contents.

Thanks for the report. That's fixed now (it was missing a header
column for the new Tags column).





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-17 04:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Ashutosh Bapat <[email protected]>
  2025-06-17 04:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tom Lane <[email protected]>
  2025-06-17 07:05     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-23 01:37       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tatsuo Ishii <[email protected]>
  2025-06-23 06:46         ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
@ 2025-06-23 06:52           ` Tatsuo Ishii <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Tatsuo Ishii @ 2025-06-23 06:52 UTC (permalink / raw)
  To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]

> On Mon, 23 Jun 2025 at 03:37, Tatsuo Ishii <[email protected]> wrote:
>> And I found "Author" column is shown as "+4207-35" which does not seem
>> to be an author name. Likewise followings columns seem to show
>> inappropriate contents.
> 
> Thanks for the report. That's fixed now (it was missing a header
> column for the new Tags column).

Thanks for the prompt response. I confirmed the fix.

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-17 04:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Ashutosh Bapat <[email protected]>
@ 2025-06-17 08:05   ` Jelte Fennema-Nio <[email protected]>
  1 sibling, 0 replies; 27+ messages in thread

From: Jelte Fennema-Nio @ 2025-06-17 08:05 UTC (permalink / raw)
  To: Ashutosh Bapat <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David G. Johnston <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On Tue, 17 Jun 2025 at 06:21, Ashutosh Bapat
<[email protected]> wrote:
> I like this as well. I feel "In Progress" doesn't convey that the
> column contains the dates when the CFs are active. It can be easily
> confused with "In progress" CF. How about just "Dates" or "Duration"?

I decided to overhaul the table a bit more than you suggested. I added
a "When Open" and "When In Progress" columns now. See attached
screenshot.

> > The full list of commitfests is still available at:
> > https://commitfest-test.postgresql.org/commitfest_history/ This page
> > is linked to at the bottom most link on the homepage.
> >
>
> All patches in the current commitfest
> All patches in the open commitfest
>
> Are those two needed anymore since the CF links are already available?

The nice feature of these links is that they are stable, so you can
bookmark them. i.e. they don't contain a specific commitfest number.

> Create a new commitfest entry - if we are creating the CFs
> automatically, is this link that useful? And it could be a button/link
> in the Commitfest section itself just before or after the table of
> relevant commitfests.

It's not about creating a new commitfest, but it's about creating a
new *entry*. In a future commitfest app update I'd like to move all
these links to a titlebar, instead of having them on the homepage.


Attachments:

  [image/png] image.png (72.1K, ../../CAGECzQTnk7z2ziwtWhBrfaReM6jW2JLAz+B6qrKMOF=-h0RAiQ@mail.gmail.com/2-image.png)
  download | view image

^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
@ 2025-06-18 03:58 ` Peter Eisentraut <[email protected]>
  2025-06-18 07:11   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  4 siblings, 1 reply; 27+ messages in thread

From: Peter Eisentraut @ 2025-06-18 03:58 UTC (permalink / raw)
  To: Jelte Fennema-Nio <[email protected]>; PostgreSQL Hackers <[email protected]>; +Cc: David G. Johnston <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On 16.06.25 14:47, Jelte Fennema-Nio wrote:
> And our already well-known commitfests for PG19 will be called as follows:
> 
> - PG19-1 (previously 2025-07)
> - PG19-2 (previously 2025-09)
> - PG19-3 (previously 2025-11)
> - PG19-4 (previously 2026-01)
> - PG19-Final (previously 2026-03)
> 
> The dates will be the same, the name will simply not be of the
> {year}-{month} format anymore. The actual dates will still be easily
> visible though. So the intent is that no-one loses information, but
> instead people will gain information because it's clear what Postgres
> version a commitfest is about.

Can you explain the motivation for this change a bit more?

I think I kind of like the calendar hints that the previous naming 
gives.  You can estimate how long ago something was or how long you 
still have to finish or prepare something.  The release number isn't 
that meaningful, and the numbering withing the release less so.

Also, I wonder if this scheme would cause confusion about the question, 
when and where am I allowed to submit patches for PG20?  Would that go 
into, say, PG19-4 or into PG20-Drafts?

Actually, even as I'm typing this message, I'm mentally confusing PG19-3 
with "March".  The number "3" just has these connotations of aaah, 
better get it done. ;-)





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-18 03:58 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
@ 2025-06-18 07:11   ` Jelte Fennema-Nio <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Jelte Fennema-Nio @ 2025-06-18 07:11 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David G. Johnston <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On Wed, 18 Jun 2025 at 05:58, Peter Eisentraut <[email protected]> wrote:
> Can you explain the motivation for this change a bit more?

A few main reasons (from important to unimportant):
1. For new/irregular contributors the old names were really not
obvious IMO. It took me 3+ years to get the mental association with
March, that that was the final one for the release.
2. Looking up a historic patch from the mailinglist in the CF app,
should now give you a good idea what PG version it got in. (e.g. Oh,
AIO support got committed in PG18-Final)
3. The March commitfest is actually not just March, it ends at the
feature freeze. Which again is not obvious for many irregular users.
4. For the Draft CFs we needed a new naming scheme, and this aligns
nicely with it.
5. It's a little bit shorter

> I think I kind of like the calendar hints that the previous naming
> gives.

To be clear, this new naming scheme is definitely up for discussion.
It's easily changeable/revertible/tunable. However, unless there's an
overwhelming negative response to this email thread about this, I
think it's worth trying out. If it's not to people's liking after
trying it out for a while (e.g. after one or two commitfests), then we
can still easily change the names back to the old scheme (even of
already created/closed commitfests).

> You can estimate how long ago something was or how long you
> still have to finish or prepare something.  The release number isn't
> that meaningful, and the numbering withing the release less so.

To be clear, I did not intend to make this harder to find out.. The
dates are still visible on the homepage, just next to the name instead
of in the name. I realized now, they are indeed missing in a few other
places. Specifically the title of a commitfest page, and the patch
page. I fixed those now, if you find other places please let me know.

> Also, I wonder if this scheme would cause confusion about the question,
> when and where am I allowed to submit patches for PG20?  Would that go
> into, say, PG19-4 or into PG20-Drafts?

I don't think this will be a problem in practice. PG20-Drafts and
PG20-1 will open at the same time: 1st of March. Before that time
people will only be able to submit to PG19-Drafts and PG19-X (with X
depending on the time of year).

> Actually, even as I'm typing this message, I'm mentally confusing PG19-3
> with "March".  The number "3" just has these connotations of aaah,
> better get it done. ;-)

Yeah, I totally get that, there's definitely some trained stress about
the number 3 for me too. But as explained in my number one reason for
the change, new users don't have that stress yet. I think Final has
those stressful connotations more naturally (and I expect getting used
to Final shouldn't be too hard for you either).





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
@ 2025-06-20 13:21 ` Peter Eisentraut <[email protected]>
  2025-06-20 14:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
  4 siblings, 1 reply; 27+ messages in thread

From: Peter Eisentraut @ 2025-06-20 13:21 UTC (permalink / raw)
  To: Jelte Fennema-Nio <[email protected]>; PostgreSQL Hackers <[email protected]>; +Cc: David G. Johnston <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On 16.06.25 14:47, Jelte Fennema-Nio wrote:
> # Draft CF
> 
> There's now an additional Draft CF. People can move patches there as a
> way of not forgetting about them. CFBot will rerun these patches less
> frequently (exact behaviour TBD). Draft CFs are never "In Progress"
> and are open until the final CF of the release cycle becomes "In
> Progress". So PG19-Drafts will close on February 28th 2026, and at the
> same moment PG20-Drafts will be opened.

Can we clarify the expectations around this?

In some early discussions, I had heard this being talked about as a 
"parking lot".  You can put patches there so that they get CI runs, but 
no one else is really expected to pay attention to them.  Makes sense.

But many patches that are routinely submitted to normal commit fests are 
really drafts, in that they are in an early phase of development but 
still need feedback.

I sense there could be some confusion whether such draft patches should 
go into the regular commit fest or the draft commit fest, and then also 
when they should move between them.






^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-20 13:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
@ 2025-06-20 14:41   ` David G. Johnston <[email protected]>
  2025-06-22 13:47     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: David G. Johnston @ 2025-06-20 14:41 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; PostgreSQL Hackers <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On Friday, June 20, 2025, Peter Eisentraut <[email protected]> wrote:

> On 16.06.25 14:47, Jelte Fennema-Nio wrote:
>
>> # Draft CF
>>
>> There's now an additional Draft CF. People can move patches there as a
>> way of not forgetting about them. CFBot will rerun these patches less
>> frequently (exact behaviour TBD). Draft CFs are never "In Progress"
>> and are open until the final CF of the release cycle becomes "In
>> Progress". So PG19-Drafts will close on February 28th 2026, and at the
>> same moment PG20-Drafts will be opened.
>>
>
> Can we clarify the expectations around this?
>
> In some early discussions, I had heard this being talked about as a
> "parking lot".  You can put patches there so that they get CI runs, but no
> one else is really expected to pay attention to them.  Makes sense.
>
> But many patches that are routinely submitted to normal commit fests are
> really drafts, in that they are in an early phase of development but still
> need feedback.
>
> I sense there could be some confusion whether such draft patches should go
> into the regular commit fest or the draft commit fest, and then also when
> they should move between them.
>

Draft CF patches with “Needs Review” are looking for feedback from others
or otherwise aid in development for a patch that isn’t ready to be
committed even if said review turns up nothing or is otherwise fully
resolved.  Patches in Drafts are never marked Ready to Commit, they get
moved to Open first.

It will be nice if people spend time providing reviews/feedback to patches
in Drafts when requested.

It’s purely the author’s judgement on the readiness of the patch, whether
absent our policy they would mark it ready to commit or not.  If they
believe it is it should be moved to open, if no, it should remain in
drafts.  This is mostly like what happens today but with a clear
delineation between reviews to help and reviews to approve commit-ability.

Otherwise, it’s a place where author patches can sit without having to be
bumped to the next cf every other month and where an author patch can be
ignored by everyone else not authoring it.

David J.


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-20 13:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
  2025-06-20 14:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
@ 2025-06-22 13:47     ` Peter Eisentraut <[email protected]>
  2025-06-22 14:21       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
  2025-06-22 20:55       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  0 siblings, 2 replies; 27+ messages in thread

From: Peter Eisentraut @ 2025-06-22 13:47 UTC (permalink / raw)
  To: David G. Johnston <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; PostgreSQL Hackers <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On 20.06.25 16:41, David G. Johnston wrote:
>     I sense there could be some confusion whether such draft patches
>     should go into the regular commit fest or the draft commit fest, and
>     then also when they should move between them.
> 
> Draft CF patches with “Needs Review” are looking for feedback from 
> others or otherwise aid in development for a patch that isn’t ready to 
> be committed even if said review turns up nothing or is otherwise fully 
> resolved.  Patches in Drafts are never marked Ready to Commit, they get 
> moved to Open first.
> 
> It will be nice if people spend time providing reviews/feedback to 
> patches in Drafts when requested.
> 
> It’s purely the author’s judgement on the readiness of the patch, 
> whether absent our policy they would mark it ready to commit or not.  If 
> they believe it is it should be moved to open, if no, it should remain 
> in drafts.  This is mostly like what happens today but with a clear 
> delineation between reviews to help and reviews to approve commit-ability.
> 
> Otherwise, it’s a place where author patches can sit without having to 
> be bumped to the next cf every other month and where an author patch can 
> be ignored by everyone else not authoring it.

I don't know about this.  This could become an ongoing source of 
confusion, without any clear benefit.  Either the draft and the "real" 
commitfest are going to be indistinguishable, because they are just two 
places to look for stuff to review in various phases of maturity.  Or 
the draft commitfest is just not going to get any attention, which will 
be annoying for those who put things there hoping to get attention.






^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-20 13:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
  2025-06-20 14:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
  2025-06-22 13:47     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
@ 2025-06-22 14:21       ` David G. Johnston <[email protected]>
  2025-06-22 16:23         ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Vik Fearing <[email protected]>
  1 sibling, 1 reply; 27+ messages in thread

From: David G. Johnston @ 2025-06-22 14:21 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; PostgreSQL Hackers <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On Sunday, June 22, 2025, Peter Eisentraut <[email protected]> wrote:

> On 20.06.25 16:41, David G. Johnston wrote:
>
>>     I sense there could be some confusion whether such draft patches
>>     should go into the regular commit fest or the draft commit fest, and
>>     then also when they should move between them.
>>
>> Draft CF patches with “Needs Review” are looking for feedback from others
>> or otherwise aid in development for a patch that isn’t ready to be
>> committed even if said review turns up nothing or is otherwise fully
>> resolved.  Patches in Drafts are never marked Ready to Commit, they get
>> moved to Open first.
>>
>> It will be nice if people spend time providing reviews/feedback to
>> patches in Drafts when requested.
>>
>> It’s purely the author’s judgement on the readiness of the patch, whether
>> absent our policy they would mark it ready to commit or not.  If they
>> believe it is it should be moved to open, if no, it should remain in
>> drafts.  This is mostly like what happens today but with a clear
>> delineation between reviews to help and reviews to approve commit-ability.
>>
>> Otherwise, it’s a place where author patches can sit without having to be
>> bumped to the next cf every other month and where an author patch can be
>> ignored by everyone else not authoring it.
>>
>
> I don't know about this.  This could become an ongoing source of
> confusion, without any clear benefit.  Either the draft and the "real"
> commitfest are going to be indistinguishable, because they are just two
> places to look for stuff to review in various phases of maturity.  Or the
> draft commitfest is just not going to get any attention, which will be
> annoying for those who put things there hoping to get attention.
>
>
Yes, more experienced people have to want to help people who can’t just get
a patch ready to commit on their own.  As opposed to only reviewing things
they expect to perform the formality of the review to make it ready to
commit.  The tooling help distinguish those cases if used properly.  But
people have to choose to do the things it encourages/enables.

If one performs a review of a non-draft and it isn’t close to ready,
encourage the author to move it into drafts as part of a teaching moment of
how their expectations of done-ness and yours differ.

We aren’t going to get 100% accuracy here but it’s is better information
that intends to address the complaint that what we had was not fit for
purpose because the information it provided was insufficient.  Tags get
even more granular while this provides high-level draft/non-draft
delineation where drafts don’t have to keep being shuffled around.  Review
Need still needs review no matter where it is.  That doesn’t change.

David J.


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-20 13:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
  2025-06-20 14:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
  2025-06-22 13:47     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
  2025-06-22 14:21       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
@ 2025-06-22 16:23         ` Vik Fearing <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Vik Fearing @ 2025-06-22 16:23 UTC (permalink / raw)
  To: David G. Johnston <[email protected]>; Peter Eisentraut <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; PostgreSQL Hackers <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>


On 22/06/2025 16:21, David G. Johnston wrote:
> On Sunday, June 22, 2025, Peter Eisentraut <[email protected]> wrote:
>
>     On 20.06.25 16:41, David G. Johnston wrote:
>
>             I sense there could be some confusion whether such draft
>         patches
>             should go into the regular commit fest or the draft commit
>         fest, and
>             then also when they should move between them.
>
>         Draft CF patches with “Needs Review” are looking for feedback
>         from others or otherwise aid in development for a patch that
>         isn’t ready to be committed even if said review turns up
>         nothing or is otherwise fully resolved.  Patches in Drafts are
>         never marked Ready to Commit, they get moved to Open first.
>
>         It will be nice if people spend time providing
>         reviews/feedback to patches in Drafts when requested.
>
>         It’s purely the author’s judgement on the readiness of the
>         patch, whether absent our policy they would mark it ready to
>         commit or not.  If they believe it is it should be moved to
>         open, if no, it should remain in drafts.  This is mostly like
>         what happens today but with a clear delineation between
>         reviews to help and reviews to approve commit-ability.
>
>         Otherwise, it’s a place where author patches can sit without
>         having to be bumped to the next cf every other month and where
>         an author patch can be ignored by everyone else not authoring it.
>
>
>     I don't know about this.  This could become an ongoing source of
>     confusion, without any clear benefit.  Either the draft and the
>     "real" commitfest are going to be indistinguishable, because they
>     are just two places to look for stuff to review in various phases
>     of maturity.  Or the draft commitfest is just not going to get any
>     attention, which will be annoying for those who put things there
>     hoping to get attention.
>
>
> Yes, more experienced people have to want to help people who can’t 
> just get a patch ready to commit on their own.  As opposed to only 
> reviewing things they expect to perform the formality of the review to 
> make it ready to commit.  The tooling help distinguish those cases if 
> used properly.  But people have to choose to do the things it 
> encourages/enables.
>
> If one performs a review of a non-draft and it isn’t close to ready, 
> encourage the author to move it into drafts as part of a teaching 
> moment of how their expectations of done-ness and yours differ.
>
> We aren’t going to get 100% accuracy here but it’s is better 
> information that intends to address the complaint that what we had was 
> not fit for purpose because the information it provided was 
> insufficient.  Tags get even more granular while this provides 
> high-level draft/non-draft delineation where drafts don’t have to keep 
> being shuffled around.  Review Need still needs review no matter where 
> it is.  That doesn’t change.


+1

-- 

Vik Fearing


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-20 13:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
  2025-06-20 14:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
  2025-06-22 13:47     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
@ 2025-06-22 20:55       ` Jelte Fennema-Nio <[email protected]>
  1 sibling, 0 replies; 27+ messages in thread

From: Jelte Fennema-Nio @ 2025-06-22 20:55 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: David G. Johnston <[email protected]>; PostgreSQL Hackers <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On Sun, 22 Jun 2025 at 15:47, Peter Eisentraut <[email protected]> wrote:
> I don't know about this.  This could become an ongoing source of
> confusion, without any clear benefit.  Either the draft and the "real"
> commitfest are going to be indistinguishable, because they are just two
> places to look for stuff to review in various phases of maturity.  Or
> the draft commitfest is just not going to get any attention, which will
> be annoying for those who put things there hoping to get attention.

I think I agree with this. We decided to go for the "Draft" name to
align with GitHub terminology. But if our usage of it is different
than how people often use the feature in GitHub it seems better to
have a different name.

How about we rename "Drafts" to "Parking Lot" (as originally proposed
for the name) and add a "Draft" tag as an option to handle the "this
is not finished, but I'd love some feedback anyway" situation?





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
@ 2025-06-22 20:50 ` Jelte Fennema-Nio <[email protected]>
  2025-06-25 17:56   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
  4 siblings, 1 reply; 27+ messages in thread

From: Jelte Fennema-Nio @ 2025-06-22 20:50 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>; +Cc: David G. Johnston <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On Mon, 16 Jun 2025 at 14:47, Jelte Fennema-Nio <[email protected]> wrote:
>
> The new PG19 development cycle is starting soon. So that seemed like a
> good excuse to make some big improvements to the commitfest app.

These changes have now been deployed to production. Please report any
problems, either as a reply or as a github issue.





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-22 20:50 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
@ 2025-06-25 17:56   ` Aleksander Alekseev <[email protected]>
  2025-06-25 18:23     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Aleksander Alekseev @ 2025-06-25 17:56 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; Jacob Champion <[email protected]>; David G. Johnston <[email protected]>; Florents Tselai <[email protected]>

Hi,

> > The new PG19 development cycle is starting soon. So that seemed like a
> > good excuse to make some big improvements to the commitfest app.
>
> These changes have now been deployed to production. Please report any
> problems, either as a reply or as a github issue.

Firstly, many thanks for working on this.

I don't see a button that would allow me to add a patch to PG19-1
(2025-07-01 - 2025-07-31). I tried to log out and log in but it didn't
change anything. Is it a bug or do I miss something?

-- 
Best regards,
Aleksander Alekseev





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-22 20:50 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-25 17:56   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
@ 2025-06-25 18:23     ` David G. Johnston <[email protected]>
  2025-06-25 18:28       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: David G. Johnston @ 2025-06-25 18:23 UTC (permalink / raw)
  To: Aleksander Alekseev <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Jelte Fennema-Nio <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On Wed, Jun 25, 2025 at 10:56 AM Aleksander Alekseev <
[email protected]> wrote:

> Hi,
>
> > > The new PG19 development cycle is starting soon. So that seemed like a
> > > good excuse to make some big improvements to the commitfest app.
> >
> > These changes have now been deployed to production. Please report any
> > problems, either as a reply or as a github issue.
>
> Firstly, many thanks for working on this.
>
> I don't see a button that would allow me to add a patch to PG19-1
> (2025-07-01 - 2025-07-31). I tried to log out and log in but it didn't
> change anything. Is it a bug or do I miss something?
>
>
Fourth link down from the top of the link section - "Create a new
commitfest entry"

Adds it to 19-1; need to move it to Drafts if that is where it belongs.

David J.


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-22 20:50 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-25 17:56   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
  2025-06-25 18:23     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
@ 2025-06-25 18:28       ` Aleksander Alekseev <[email protected]>
  2025-06-25 19:59         ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Aleksander Alekseev @ 2025-06-25 18:28 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>; +Cc: David G. Johnston <[email protected]>; Jelte Fennema-Nio <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

Hi,

> Fourth link down from the top of the link section - "Create a new commitfest entry"
>
> Adds it to 19-1; need to move it to Drafts if that is where it belongs.

Found it. It was moved to the "Your personal dashboard" page.
Previously I used the CF page thus I couldn't find it.

Many thanks.

-- 
Best regards,
Aleksander Alekseev





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-22 20:50 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-25 17:56   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
  2025-06-25 18:23     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
  2025-06-25 18:28       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
@ 2025-06-25 19:59         ` Jelte Fennema-Nio <[email protected]>
  2025-06-25 21:50           ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Jelte Fennema-Nio @ 2025-06-25 19:59 UTC (permalink / raw)
  To: Aleksander Alekseev <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David G. Johnston <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On Wed, 25 Jun 2025 at 20:29, Aleksander Alekseev
<[email protected]> wrote:
>
> Hi,
>
> > Fourth link down from the top of the link section - "Create a new commitfest entry"
> >
> > Adds it to 19-1; need to move it to Drafts if that is where it belongs.
>
> Found it. It was moved to the "Your personal dashboard" page.
> Previously I used the CF page thus I couldn't find it.

Ugh... Turns out it was a bug, there definitely should be a "New
patch" button on both the 19-1 and on the Drafts page. And there
was... but only if you were logged in as a staff user.


I had changed a field name to from "isopen" to "is_open" and forgot to
change the usage in that template. Very annoying that django templates
not complaining about missing attributes...





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-22 20:50 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
  2025-06-25 17:56   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
  2025-06-25 18:23     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
  2025-06-25 18:28       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
  2025-06-25 19:59         ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
@ 2025-06-25 21:50           ` Aleksander Alekseev <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Aleksander Alekseev @ 2025-06-25 21:50 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; David G. Johnston <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

Hi,

> Ugh... Turns out it was a bug, there definitely should be a "New
> patch" button on both the 19-1 and on the Drafts page. And there
> was... but only if you were logged in as a staff user.

There is now a "New patch" button on the CF entry page. Many thanks!

> I had changed a field name to from "isopen" to "is_open" and forgot to
> change the usage in that template. Very annoying that django templates
> not complaining about missing attributes...

Time to rewrite CF application in Haskell and Yesod? :D (Not
realistic, I know...)

-- 
Best regards,
Aleksander Alekseev





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
  2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
@ 2025-06-25 00:01 ` David G. Johnston <[email protected]>
  4 siblings, 0 replies; 27+ messages in thread

From: David G. Johnston @ 2025-06-25 00:01 UTC (permalink / raw)
  To: Jelte Fennema-Nio <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Jacob Champion <[email protected]>; Florents Tselai <[email protected]>

On Mon, Jun 16, 2025 at 5:47 AM Jelte Fennema-Nio <[email protected]>
wrote:

> The new PG19 development cycle is starting soon. So that seemed like a
> good excuse to make some big improvements to the commitfest app. My
> plan is to deploy these changes on the 30th of June. So that we can
> start the new cycle fresh with these changes. As always feedback on
> these changes is very welcome. The below will contain some links to
> the staging environment both username and password for the http auth
> are "pgtest".
>
>
Been using this a bit today:

Due to using "Open" for Drafts the tag colors for Pg19-Drafts and PG19-1
are identical.  They need to be different.

When creating a new patch it should either be placed in Drafts first, and
then moved if appropriate, or the user should choose (and be given an
explanation of the decision factors behind that choice) during creation.

The "Help -" tag coloring probably should indicate some kind of blocker, so
a hot color like red/orange/yellow.  Presently it is green, which for many
is an "All good" color.

David J.


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

end of thread, other threads:[~2025-06-25 21:50 UTC | newest]

Thread overview: 27+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-07-08 06:08 [PATCH 1/2] Be strict in numeric parameters on command line Kyotaro Horiguchi <[email protected]>
2021-07-08 06:08 [PATCH v3 1/3] Be strict in numeric parameters on command line Kyotaro Horiguchi <[email protected]>
2021-07-08 06:08 [PATCH v2 1/2] Be strict in numeric parameters on command line Kyotaro Horiguchi <[email protected]>
2021-07-08 06:08 [PATCH v3 1/3] Be strict in numeric parameters on command line Kyotaro Horiguchi <[email protected]>
2025-06-16 12:47 Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
2025-06-17 04:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Ashutosh Bapat <[email protected]>
2025-06-17 04:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tom Lane <[email protected]>
2025-06-17 07:05     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
2025-06-23 01:37       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tatsuo Ishii <[email protected]>
2025-06-23 06:46         ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
2025-06-23 06:52           ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Tatsuo Ishii <[email protected]>
2025-06-17 08:05   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
2025-06-18 03:58 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
2025-06-18 07:11   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
2025-06-20 13:21 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
2025-06-20 14:41   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
2025-06-22 13:47     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Peter Eisentraut <[email protected]>
2025-06-22 14:21       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
2025-06-22 16:23         ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Vik Fearing <[email protected]>
2025-06-22 20:55       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
2025-06-22 20:50 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
2025-06-25 17:56   ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
2025-06-25 18:23     ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>
2025-06-25 18:28       ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
2025-06-25 19:59         ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Jelte Fennema-Nio <[email protected]>
2025-06-25 21:50           ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close Aleksander Alekseev <[email protected]>
2025-06-25 00:01 ` Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close David G. Johnston <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox