pg_restore: Remove unnecessary strlen() calls in options parsing.

Unlike pg_dump and pg_dumpall, pg_restore first checks whether the
argument passed to --format, --host, and --port is empty before
setting the corresponding variable.  Consequently, pg_restore does
not error if given an empty format name, whereas pg_dump and
pg_dumpall do.  Empty arguments for --host and --port are ignored
by all three applications, so this commit produces no functionality
changes there.  This behavior should perhaps be reconsidered, but
that is left as a future exercise.  As with other recent changes to
option handling for these applications (commits b2898baaf7,
7c8280eeb5, and be0d0b457c), no back-patch.

Author: Mahendra Singh Thalor <mahi6run@gmail.com>
Reviewed-by: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Discussion: https://postgr.es/m/CAKYtNApkh%3DVy2DpNRCnEJmPpxNuksbAh_QBav%3D2fLmVjBhGwFw%40mail.gmail.com

diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 91efe30565094951db78ded7672ed790d58301f2..f016b336308921e7d6594441dcd334aa5f5cf328 100644 (file)
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -225,16 +225,14 @@ main(int argc, char **argv)
                                opts->filename = pg_strdup(optarg);
                                break;
                        case 'F':
-                               if (strlen(optarg) != 0)
-                                       opts->formatName = pg_strdup(optarg);
+                               opts->formatName = pg_strdup(optarg);
                                break;
                        case 'g':
                                /* restore only global sql commands. */
                                globals_only = true;
                                break;
                        case 'h':
-                               if (strlen(optarg) != 0)
-                                       opts->cparams.pghost = pg_strdup(optarg);
+                               opts->cparams.pghost = pg_strdup(optarg);
                                break;
                        case 'j':                       /* number of restore jobs */
                                if (!option_parse_int(optarg, "-j/--jobs", 1,
@@ -264,8 +262,7 @@ main(int argc, char **argv)
                                break;
 
                        case 'p':
-                               if (strlen(optarg) != 0)
-                                       opts->cparams.pgport = pg_strdup(optarg);
+                               opts->cparams.pgport = pg_strdup(optarg);
                                break;
                        case 'R':
                                /* no-op, still accepted for backwards compatibility */

diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 86eec1b00641b34e21516565f3555759c4a64058..509f4f9ce7dfc6e39397663f258a32698855c7f2 100644 (file)
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -204,7 +204,12 @@ command_fails_like(
 command_fails_like(
        [ 'pg_restore', '-f -', '-F', 'garbage' ],
        qr/\Qpg_restore: error: unrecognized archive format "garbage";\E/,
-       'pg_dump: unrecognized archive format');
+       'pg_restore: unrecognized archive format');
+
+command_fails_like(
+       [ 'pg_restore', '-f -', '-F', '' ],
+       qr/\Qpg_restore: error: unrecognized archive format "";\E/,
+       'pg_restore: empty archive format');
 
 command_fails_like(
        [ 'pg_dump', '--on-conflict-do-nothing' ],