libpq_pipeline: Must strdup(optarg) to avoid crash

alvherre · alvherre · commit dde1a35aee62 · 2021-04-01T10:26:20.000-03:00
I forgot to strdup() when processing argv[]. Apparently many platforms hide this mistake from users, but in those that don't you may get a program crash. Repair. Per buildfarm member drongo, which is the only one in all the buildfarm manifesting a problem here. While at it, move "numrows" processing out of the line of special cases, and make it getopt's -r instead. (A similar thing could be done to 'conninfo', but current use of the program doesn't warrant spending time on that -- nowhere else we use conninfo in so simplistic a manner.) Discussion: https://postgr.es/m/20210401124850.GA19247@alvherre.pgsql
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -725,7 +725,7 @@ test_pipelined_insert(PGconn *conn, int n_rows)
 			{
 				snprintf(insert_param_0, MAXINTLEN, "%d", rows_to_send);
 				snprintf(insert_param_1, MAXINT8LEN, "%lld",
-						 (long long) rows_to_send);
+						 (1L << 62) + (long long) rows_to_send);
 
 				if (PQsendQueryPrepared(conn, "my_insert",
 										2, insert_params, NULL, NULL, 0) == 1)
@@ -1227,9 +1227,10 @@ usage(const char *progname)
 	fprintf(stderr, "%s tests libpq's pipeline mode.\n\n", progname);
 	fprintf(stderr, "Usage:\n");
 	fprintf(stderr, "  %s [OPTION] tests\n", progname);
-	fprintf(stderr, "  %s [OPTION] TESTNAME [CONNINFO [NUMBER_OF_ROWS]\n", progname);
+	fprintf(stderr, "  %s [OPTION] TESTNAME [CONNINFO]\n", progname);
 	fprintf(stderr, "\nOptions:\n");
 	fprintf(stderr, "  -t TRACEFILE       generate a libpq trace to TRACEFILE\n");
+	fprintf(stderr, "  -r NUMROWS         use NUMROWS as the test size\n");
 }
 
 static void
@@ -1256,19 +1257,29 @@ main(int argc, char **argv)
 	PGresult   *res;
 	int			c;
 
-	while ((c = getopt(argc, argv, "t:")) != -1)
+	while ((c = getopt(argc, argv, "t:r:")) != -1)
 	{
 		switch (c)
 		{
 			case 't':			/* trace file */
 				tracefile = pg_strdup(optarg);
 				break;
+			case 'r':			/* numrows */
+				errno = 0;
+				numrows = strtol(optarg, NULL, 10);
+				if (errno != 0 || numrows <= 0)
+				{
+					fprintf(stderr, "couldn't parse \"%s\" as a positive integer\n",
+							optarg);
+					exit(1);
+				}
+				break;
 		}
 	}
 
 	if (optind < argc)
 	{
-		testname = argv[optind];
+		testname = pg_strdup(argv[optind]);
 		optind++;
 	}
 	else
@@ -1285,18 +1296,7 @@ main(int argc, char **argv)
 
 	if (optind < argc)
 	{
-		conninfo = argv[optind];
-		optind++;
-	}
-	if (optind < argc)
-	{
-		errno = 0;
-		numrows = strtol(argv[optind], NULL, 10);
-		if (errno != 0 || numrows <= 0)
-		{
-			fprintf(stderr, "couldn't parse \"%s\" as a positive integer\n", argv[optind]);
-			exit(1);
-		}
+		conninfo = pg_strdup(argv[optind]);
 		optind++;
 	}
 
diff --git a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl b/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
@@ -22,7 +22,7 @@
 
 for my $testname (@tests)
 {
-	my @extraargs = ();
+	my @extraargs = ('-r', $numrows);
 	my $cmptrace  = grep(/^$testname$/,
 		qw(simple_pipeline multi_pipelines prepared singlerow
 		  pipeline_abort transaction disallowed_in_pipeline)) > 0;
@@ -38,8 +38,7 @@
 	$node->command_ok(
 		[
 			'libpq_pipeline', @extraargs,
-			$testname,        $node->connstr('postgres'),
-			$numrows
+			$testname,        $node->connstr('postgres')
 		],
 		"libpq_pipeline $testname");
 
