From dde1a35aee6266dc8105717275335c46cd2b3650 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 1 Apr 2021 10:04:45 -0300 Subject: [PATCH] libpq_pipeline: Must strdup(optarg) to avoid crash 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 --- .../modules/libpq_pipeline/libpq_pipeline.c | 32 +++++++++---------- .../libpq_pipeline/t/001_libpq_pipeline.pl | 5 ++- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c index 7aa78662653..4fc35cacf8f 100644 --- 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 index f8e39b58131..4819dbd8495 100644 --- 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 @@ mkdir "$TestLib::tmp_check/traces"; 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 @@ for my $testname (@tests) $node->command_ok( [ 'libpq_pipeline', @extraargs, - $testname, $node->connstr('postgres'), - $numrows + $testname, $node->connstr('postgres') ], "libpq_pipeline $testname");