Hi all, As $subject says, pg_test_fsync and pg_test_timing don't really check the range of option values specified. It is possible for example to make pg_test_fsync run an infinite amount of time, and pg_test_timing does not handle overflows with --duration at all.
These are far from being critical issues, but let's fix them at least on HEAD. So, please see the attached, where I have also added some basic TAP tests for both tools. Thanks, -- Michael
diff --git a/src/bin/pg_test_fsync/.gitignore b/src/bin/pg_test_fsync/.gitignore
index f3b5932498..5eb5085f45 100644
--- a/src/bin/pg_test_fsync/.gitignore
+++ b/src/bin/pg_test_fsync/.gitignore
@@ -1 +1,3 @@
/pg_test_fsync
+
+/tmp_check/
diff --git a/src/bin/pg_test_fsync/Makefile b/src/bin/pg_test_fsync/Makefile
index 7632c94eb7..c4f9ae0664 100644
--- a/src/bin/pg_test_fsync/Makefile
+++ b/src/bin/pg_test_fsync/Makefile
@@ -22,6 +22,12 @@ install: all installdirs
installdirs:
$(MKDIR_P) '$(DESTDIR)$(bindir)'
+check:
+ $(prove_check)
+
+installcheck:
+ $(prove_installcheck)
+
uninstall:
rm -f '$(DESTDIR)$(bindir)/pg_test_fsync$(X)'
diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c
index 6e47293123..e56b42bcf8 100644
--- a/src/bin/pg_test_fsync/pg_test_fsync.c
+++ b/src/bin/pg_test_fsync/pg_test_fsync.c
@@ -5,6 +5,7 @@
#include "postgres_fe.h"
+#include <limits.h>
#include <sys/stat.h>
#include <sys/time.h>
#include <fcntl.h>
@@ -174,6 +175,12 @@ handle_args(int argc, char *argv[])
case 's':
secs_per_test = atoi(optarg);
+ if (secs_per_test <= 0 || errno == ERANGE)
+ {
+ pg_log_error("%s must be in range %d..%d",
+ "--secs-per-test", 1, INT_MAX);
+ exit(1);
+ }
break;
default:
diff --git a/src/bin/pg_test_fsync/t/001_basic.pl b/src/bin/pg_test_fsync/t/001_basic.pl
new file mode 100644
index 0000000000..cc5517cb06
--- /dev/null
+++ b/src/bin/pg_test_fsync/t/001_basic.pl
@@ -0,0 +1,21 @@
+use strict;
+use warnings;
+
+use Config;
+use TestLib;
+use Test::More tests => 10;
+
+#########################################
+# Basic checks
+
+program_help_ok('pg_test_fsync');
+program_version_ok('pg_test_fsync');
+program_options_handling_ok('pg_test_fsync');
+
+#########################################
+# Test invalid option combinations
+
+command_fails_like(
+ [ 'pg_test_fsync', '--secs-per-test', '0' ],
+ qr/\Qpg_test_fsync: error: --secs-per-test must be in range 1..2147483647\E/,
+ 'pg_test_fsync: --secs-per-test must be in range 1..2147483647');
diff --git a/src/bin/pg_test_timing/.gitignore b/src/bin/pg_test_timing/.gitignore
index f6c664c765..e5aac2ab12 100644
--- a/src/bin/pg_test_timing/.gitignore
+++ b/src/bin/pg_test_timing/.gitignore
@@ -1 +1,3 @@
/pg_test_timing
+
+/tmp_check/
diff --git a/src/bin/pg_test_timing/Makefile b/src/bin/pg_test_timing/Makefile
index 334d6ff5c0..52994b4103 100644
--- a/src/bin/pg_test_timing/Makefile
+++ b/src/bin/pg_test_timing/Makefile
@@ -22,6 +22,12 @@ install: all installdirs
installdirs:
$(MKDIR_P) '$(DESTDIR)$(bindir)'
+check:
+ $(prove_check)
+
+installcheck:
+ $(prove_installcheck)
+
uninstall:
rm -f '$(DESTDIR)$(bindir)/pg_test_timing$(X)'
diff --git a/src/bin/pg_test_timing/pg_test_timing.c b/src/bin/pg_test_timing/pg_test_timing.c
index e14802372b..d369d32781 100644
--- a/src/bin/pg_test_timing/pg_test_timing.c
+++ b/src/bin/pg_test_timing/pg_test_timing.c
@@ -6,6 +6,8 @@
#include "postgres_fe.h"
+#include <limits.h>
+
#include "getopt_long.h"
#include "portability/instr_time.h"
@@ -69,6 +71,12 @@ handle_args(int argc, char *argv[])
{
case 'd':
test_duration = atoi(optarg);
+ if (test_duration <= 0 || errno == ERANGE)
+ {
+ fprintf(stderr, _("%s: %s must be in range %d..%d\n"),
+ progname, "--duration", 1, INT_MAX);
+ exit(1);
+ }
break;
default:
@@ -89,22 +97,11 @@ handle_args(int argc, char *argv[])
exit(1);
}
- if (test_duration > 0)
- {
- printf(ngettext("Testing timing overhead for %d second.\n",
- "Testing timing overhead for %d seconds.\n",
- test_duration),
- test_duration);
- }
- else
- {
- fprintf(stderr,
- _("%s: duration must be a positive integer (duration is \"%d\")\n"),
- progname, test_duration);
- fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
- progname);
- exit(1);
- }
+
+ printf(ngettext("Testing timing overhead for %d second.\n",
+ "Testing timing overhead for %d seconds.\n",
+ test_duration),
+ test_duration);
}
static uint64
diff --git a/src/bin/pg_test_timing/t/001_basic.pl b/src/bin/pg_test_timing/t/001_basic.pl
new file mode 100644
index 0000000000..32dcfbe572
--- /dev/null
+++ b/src/bin/pg_test_timing/t/001_basic.pl
@@ -0,0 +1,21 @@
+use strict;
+use warnings;
+
+use Config;
+use TestLib;
+use Test::More tests => 10;
+
+#########################################
+# Basic checks
+
+program_help_ok('pg_test_timing');
+program_version_ok('pg_test_timing');
+program_options_handling_ok('pg_test_timing');
+
+#########################################
+# Test invalid option combinations
+
+command_fails_like(
+ [ 'pg_test_timing', '--duration', '0' ],
+ qr/\Qpg_test_timing: --duration must be in range 1..2147483647\E/,
+ 'pg_test_timing: --duration must be in range 1..2147483647');
signature.asc
Description: PGP signature
