Duplicate argv data before poptFreeContext().

2 files changed, 90 insertions, 83 deletions

diff --git a/main.c b/main.c
index d2a7b9b5..9f36904d 100644
--- a/main.c
+++ b/main.c
@@ -1381,15 +1381,6 @@ int client_run(int f_in, int f_out, pid_t pid, int argc, char *argv[])
 	return MAX(exit_code, exit_code2);
 }
 
-static void dup_argv(char *argv[])
-{
-	int i;
-
-	for (i = 0; argv[i]; i++)
-		argv[i] = strdup(argv[i]);
-}
-
-
 /* Start a client for either type of remote connection.  Work out
  * whether the arguments request a remote shell or rsyncd connection,
  * and call the appropriate connection function, then run_client.
@@ -1405,10 +1396,6 @@ static int start_client(int argc, char *argv[])
 	int ret;
 	pid_t pid;
 
-	/* Don't clobber argv[] so that ps(1) can still show the right
-	 * command line. */
-	dup_argv(argv);
-
 	if (!read_batch) { /* for read_batch, NO source is specified */
 		char *path = check_for_hostspec(argv[0], &shell_machine, &rsync_port);
 		if (path) { /* source is remote */
diff --git a/options.c b/options.c
index cfa3e1dc..ded0e7a3 100644
--- a/options.c
+++ b/options.c
@@ -200,6 +200,7 @@ int remote_option_cnt = 0;
 const char **remote_options = NULL;
 const char *checksum_choice = NULL;
 const char *compress_choice = NULL;
+static const char *empty_argv[1];
 
 int quiet = 0;
 int output_motd = 1;
@@ -1347,7 +1348,7 @@ char *alt_dest_opt(int type)
  **/
 int parse_arguments(int *argc_p, const char ***argv_p)
 {
-	static poptContext pc;
+	poptContext pc;
 	const char *arg, **argv = *argv_p;
 	int argc = *argc_p;
 	int opt, want_dest_type;
@@ -1367,10 +1368,6 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 
 	/* TODO: Call poptReadDefaultConfig; handle errors. */
 
-	/* The context leaks in case of an error, but if there's a
-	 * problem we always exit anyhow. */
-	if (pc)
-		poptFreeContext(pc);
 	pc = poptGetContext(RSYNC_NAME, argc, argv, long_options, 0);
 	if (!am_server) {
 		poptReadDefaultConfig(pc, 0);
@@ -1413,7 +1410,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 				strlcpy(err_buf,
 					"Attempt to hack rsync thwarted!\n",
 					sizeof err_buf);
-				return 0;
+				goto cleanup;
 			}
 #ifdef ICONV_OPTION
 			iconv_opt = NULL;
@@ -1459,7 +1456,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			if (tmpdir && strlen(tmpdir) >= MAXPATHLEN - 10) {
 				snprintf(err_buf, sizeof err_buf,
 					 "the --temp-dir path is WAY too long.\n");
-				return 0;
+				goto cleanup;
 			}
 
 			if (!daemon_opt) {
@@ -1469,8 +1466,16 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 				exit_cleanup(RERR_SYNTAX);
 			}
 
-			*argv_p = argv = poptGetArgs(pc);
-			*argc_p = argc = count_args(argv);
+			argv = poptGetArgs(pc);
+			argc = count_args(argv);
+			if (!argc) {
+				*argv_p = empty_argv;
+				*argc_p = 0;
+			} else if (poptDupArgv(argc, argv, argc_p, argv_p) != 0)
+				out_of_memory("parse_arguments");
+			argv = *argv_p;
+			poptFreeContext(pc);
+
 			am_starting_up = 0;
 			daemon_opt = 0;
 			am_daemon = 1;
@@ -1525,7 +1530,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 		case 'a':
 			if (refused_archive_part) {
 				create_refuse_error(refused_archive_part);
-				return 0;
+				goto cleanup;
 			}
 			if (!recurse) /* preserve recurse == 2 */
 				recurse = 1;
@@ -1595,7 +1600,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 		case 'P':
 			if (refused_partial || refused_progress) {
 				create_refuse_error(refused_partial ? refused_partial : refused_progress);
-				return 0;
+				goto cleanup;
 			}
 			do_progress = 1;
 			keep_partial = 1;
@@ -1630,7 +1635,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			if (*arg != '-') {
 				snprintf(err_buf, sizeof err_buf,
 					"Remote option must start with a dash: %s\n", arg);
-				return 0;
+				goto cleanup;
 			}
 			if (remote_option_cnt+2 >= remote_option_alloc) {
 				remote_option_alloc += 16;
@@ -1672,27 +1677,27 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			ssize_t size;
 			arg = poptGetOptArg(pc);
 			if ((size = parse_size_arg(arg, 'b', "block-size", 0, max_blength, False)) < 0)
-				return 0;
+				goto cleanup;
 			block_size = (int32)size;
 			break;
 		}
 
 		case OPT_MAX_SIZE:
 			if ((max_size = parse_size_arg(max_size_arg, 'b', "max-size", 0, -1, False)) < 0)
-				return 0;
+				goto cleanup;
 			max_size_arg = strdup(do_big_num(max_size, 0, NULL));
 			break;
 
 		case OPT_MIN_SIZE:
 			if ((min_size = parse_size_arg(min_size_arg, 'b', "min-size", 0, -1, False)) < 0)
-				return 0;
+				goto cleanup;
 			min_size_arg = strdup(do_big_num(min_size, 0, NULL));
 			break;
 
 		case OPT_BWLIMIT: {
 			ssize_t size = parse_size_arg(bwlimit_arg, 'K', "bwlimit", 512, -1, True);
 			if (size < 0)
-				return 0;
+				goto cleanup;
 			bwlimit_arg = strdup(do_big_num(size, 0, NULL));
 			bwlimit = (size + 512) / 1024;
 			break;
@@ -1721,7 +1726,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 				snprintf(err_buf, sizeof err_buf,
 					"ERROR: the %s option conflicts with the %s option\n",
 					alt_dest_opt(want_dest_type), alt_dest_opt(0));
-				return 0;
+				goto cleanup;
 			}
 			alt_dest_type = want_dest_type;
 
@@ -1729,7 +1734,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 				snprintf(err_buf, sizeof err_buf,
 					"ERROR: at most %d %s args may be specified\n",
 					MAX_BASIS_DIRS, alt_dest_opt(0));
-				return 0;
+				goto cleanup;
 			}
 			/* We defer sanitizing this arg until we know what
 			 * our destination directory is going to be. */
@@ -1742,7 +1747,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 				snprintf(err_buf, sizeof err_buf,
 					"Invalid argument passed to --chmod (%s)\n",
 					arg);
-				return 0;
+				goto cleanup;
 			}
 			break;
 
@@ -1761,11 +1766,11 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 				if (usermap_via_chown) {
 					snprintf(err_buf, sizeof err_buf,
 						"--usermap conflicts with prior --chown.\n");
-					return 0;
+					goto cleanup;
 				}
 				snprintf(err_buf, sizeof err_buf,
 					"You can only specify --usermap once.\n");
-				return 0;
+				goto cleanup;
 			}
 			usermap = (char *)poptGetOptArg(pc);
 			usermap_via_chown = False;
@@ -1777,11 +1782,11 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 				if (groupmap_via_chown) {
 					snprintf(err_buf, sizeof err_buf,
 						"--groupmap conflicts with prior --chown.\n");
-					return 0;
+					goto cleanup;
 				}
 				snprintf(err_buf, sizeof err_buf,
 					"You can only specify --groupmap once.\n");
-				return 0;
+				goto cleanup;
 			}
 			groupmap = (char *)poptGetOptArg(pc);
 			groupmap_via_chown = False;
@@ -1800,11 +1805,11 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 					if (!usermap_via_chown) {
 						snprintf(err_buf, sizeof err_buf,
 							"--chown conflicts with prior --usermap.\n");
-						return 0;
+						goto cleanup;
 					}
 					snprintf(err_buf, sizeof err_buf,
 						"You can only specify a user-affecting --chown once.\n");
-					return 0;
+					goto cleanup;
 				}
 				if (asprintf(&usermap, "*:%.*s", len, chown) < 0)
 					out_of_memory("parse_arguments");
@@ -1816,11 +1821,11 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 					if (!groupmap_via_chown) {
 						snprintf(err_buf, sizeof err_buf,
 							"--chown conflicts with prior --groupmap.\n");
-						return 0;
+						goto cleanup;
 					}
 					snprintf(err_buf, sizeof err_buf,
 						"You can only specify a group-affecting --chown once.\n");
-					return 0;
+					goto cleanup;
 				}
 				if (asprintf(&groupmap, "*:%s", arg) < 0)
 					out_of_memory("parse_arguments");
@@ -1848,7 +1853,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			snprintf(err_buf,sizeof(err_buf),
 				 "ACLs are not supported on this %s\n",
 				 am_server ? "server" : "client");
-			return 0;
+			goto cleanup;
 #endif
 
 		case 'X':
@@ -1859,7 +1864,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			snprintf(err_buf,sizeof(err_buf),
 				 "extended attributes are not supported on this %s\n",
 				 am_server ? "server" : "client");
-			return 0;
+			goto cleanup;
 #endif
 
 		case OPT_STOP_AFTER: {
@@ -1868,7 +1873,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			stop_at_utime = time(NULL);
 			if ((val = atol(arg) * 60) <= 0 || LONG_MAX - val < stop_at_utime || (long)(time_t)val != val) {
 				snprintf(err_buf, sizeof err_buf, "invalid --stop-after value: %s\n", arg);
-				return 0;
+				goto cleanup;
 			}
 			stop_at_utime += val;
 			break;
@@ -1879,11 +1884,11 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			arg = poptGetOptArg(pc);
 			if ((stop_at_utime = parse_time(arg)) == (time_t)-1) {
 				snprintf(err_buf, sizeof err_buf, "invalid --stop-at format: %s\n", arg);
-				return 0;
+				goto cleanup;
 			}
 			if (stop_at_utime <= time(NULL)) {
 				snprintf(err_buf, sizeof err_buf, "--stop-at time is not in the future: %s\n", arg);
-				return 0;
+				goto cleanup;
 			}
 			break;
 #endif
@@ -1901,7 +1906,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			else {
 				snprintf(err_buf, sizeof err_buf,
 					"--stderr mode \"%s\" is not one of errors, all, or client\n", arg);
-				return 0;
+				goto cleanup;
 			}
 			saw_stderr_opt = 1;
 			break;
@@ -1912,13 +1917,13 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			 * turned this option off. */
 			if (opt >= OPT_REFUSED_BASE) {
 				create_refuse_error(opt);
-				return 0;
+				goto cleanup;
 			}
 			snprintf(err_buf, sizeof err_buf, "%s%s: %s\n",
 				 am_server ? "on remote machine: " : "",
 				 poptBadOption(pc, POPT_BADOPTION_NOALIAS),
 				 poptStrerror(opt));
-			return 0;
+			goto cleanup;
 		}
 	}
 
@@ -1938,7 +1943,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 	if (max_alloc_arg) {
 		ssize_t size = parse_size_arg(max_alloc_arg, 'B', "max-alloc", 1024*1024, -1, True);
 		if (size < 0)
-			return 0;
+			goto cleanup;
 		max_alloc = size;
 	}
 
@@ -1952,7 +1957,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 		if (protect_args > 0) {
 			snprintf(err_buf, sizeof err_buf,
 				 "--secluded-args conflicts with --old-args.\n");
-			return 0;
+			goto cleanup;
 		}
 		protect_args = 0;
 	}
@@ -1997,7 +2002,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			do_compression = CPRES_AUTO;
 		if (do_compression && refused_compress) {
 			create_refuse_error(refused_compress);
-			return 0;
+			goto cleanup;
 		}
 	}
 
@@ -2022,7 +2027,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 		default:
 			snprintf(err_buf, sizeof err_buf,
 				"Invalid --outbuf setting -- specify N, L, or B.\n");
-			return 0;
+			goto cleanup;
 		}
 		setvbuf(stdout, (char *)NULL, mode, 0);
 	}
@@ -2050,7 +2055,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 	}
 	if (refused_no_iconv && !iconv_opt) {
 		create_refuse_error(refused_no_iconv);
-		return 0;
+		goto cleanup;
 	}
 #endif
 
@@ -2061,18 +2066,30 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 	if (orig_protect_args == 2 && am_server)
 		protect_args = orig_protect_args;
 
-	if (protect_args == 1 && am_server)
+	if (protect_args == 1 && am_server) {
+		poptFreeContext(pc);
 		return 1;
+	}
 
-	*argv_p = argv = poptGetArgs(pc);
-	*argc_p = argc = count_args(argv);
+	/* Because popt 1.19 has started to free the returned args data, we now
+	 * make a copy of the array and then do an immediate cleanup. */
+	argv = poptGetArgs(pc);
+	argc = count_args(argv);
+	if (!argc) {
+		*argv_p = empty_argv;
+		*argc_p = 0;
+	} else if (poptDupArgv(argc, argv, argc_p, argv_p) != 0)
+		out_of_memory("parse_arguments");
+	argv = *argv_p;
+	poptFreeContext(pc);
+	pc = NULL;
 
 #ifndef SUPPORT_LINKS
 	if (preserve_links && !am_sender) {
 		snprintf(err_buf, sizeof err_buf,
 			 "symlinks are not supported on this %s\n",
 			 am_server ? "server" : "client");
-		return 0;
+		goto cleanup;
 	}
 #endif
 
@@ -2081,7 +2098,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 		snprintf(err_buf, sizeof err_buf,
 			 "hard links are not supported on this %s\n",
 			 am_server ? "server" : "client");
-		return 0;
+		goto cleanup;
 	}
 #endif
 
@@ -2089,20 +2106,20 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 	if (am_root < 0 && preserve_xattrs > 1) {
 		snprintf(err_buf, sizeof err_buf,
 			 "--fake-super conflicts with -XX\n");
-		return 0;
+		goto cleanup;
 	}
 #else
 	if (am_root < 0) {
 		snprintf(err_buf, sizeof err_buf,
 			 "--fake-super requires an rsync with extended attributes enabled\n");
-		return 0;
+		goto cleanup;
 	}
 #endif
 
 	if (write_batch && read_batch) {
 		snprintf(err_buf, sizeof err_buf,
 			"--write-batch and --read-batch can not be used together\n");
-		return 0;
+		goto cleanup;
 	}
 	if (write_batch > 0 || read_batch) {
 		if (am_server) {
@@ -2121,25 +2138,25 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 	if (read_batch && files_from) {
 		snprintf(err_buf, sizeof err_buf,
 			"--read-batch cannot be used with --files-from\n");
-		return 0;
+		goto cleanup;
 	}
 	if (read_batch && remove_source_files) {
 		snprintf(err_buf, sizeof err_buf,
 			"--read-batch cannot be used with --remove-%s-files\n",
 			remove_source_files == 1 ? "source" : "sent");
-		return 0;
+		goto cleanup;
 	}
 	if (batch_name && strlen(batch_name) > MAX_BATCH_NAME_LEN) {
 		snprintf(err_buf, sizeof err_buf,
 			"the batch-file name must be %d characters or less.\n",
 			MAX_BATCH_NAME_LEN);
-		return 0;
+		goto cleanup;
 	}
 
 	if (tmpdir && strlen(tmpdir) >= MAXPATHLEN - 10) {
 		snprintf(err_buf, sizeof err_buf,
 			 "the --temp-dir path is WAY too long.\n");
-		return 0;
+		goto cleanup;
 	}
 
 	if (max_delete < 0 && max_delete != INT_MIN) {
@@ -2173,7 +2190,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 	if (delete_before + !!delete_during + delete_after > 1) {
 		snprintf(err_buf, sizeof err_buf,
 			"You may not combine multiple --delete-WHEN options.\n");
-		return 0;
+		goto cleanup;
 	}
 	if (delete_before || delete_during || delete_after)
 		delete_mode = 1;
@@ -2184,7 +2201,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 				delete_during = 1;
 			else {
 				create_refuse_error(refused_delete_before);
-				return 0;
+				goto cleanup;
 			}
 		} else if (refused_delete_during)
 			delete_before = 1;
@@ -2193,14 +2210,14 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 	if (!xfer_dirs && delete_mode) {
 		snprintf(err_buf, sizeof err_buf,
 			"--delete does not work without --recursive (-r) or --dirs (-d).\n");
-		return 0;
+		goto cleanup;
 	}
 
 	if (missing_args == 3) /* simplify if both options were specified */
 		missing_args = 2;
 	if (refused_delete && (delete_mode || missing_args == 2)) {
 		create_refuse_error(refused_delete);
-		return 0;
+		goto cleanup;
 	}
 
 	if (remove_source_files) {
@@ -2209,7 +2226,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 		 * options. */
 		if (refused_delete && am_sender) {
 			create_refuse_error(refused_delete);
-			return 0;
+			goto cleanup;
 		}
 		need_messages_from_generator = 1;
 	}
@@ -2263,7 +2280,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 		snprintf(err_buf, sizeof err_buf,
 			"--suffix cannot contain slashes: %s\n",
 			backup_suffix);
-		return 0;
+		goto cleanup;
 	}
 	if (backup_dir) {
 		size_t len;
@@ -2276,7 +2293,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 		if (len > sizeof backup_dir_buf - 128) {
 			snprintf(err_buf, sizeof err_buf,
 				"the --backup-dir path is WAY too long.\n");
-			return 0;
+			goto cleanup;
 		}
 		backup_dir_len = (int)len;
 		if (!backup_dir_len) {
@@ -2295,7 +2312,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			"--suffix cannot be empty %s\n", backup_dir_len < 0
 			? "when --backup-dir is the same as the dest dir"
 			: "without a --backup-dir");
-		return 0;
+		goto cleanup;
 	} else if (make_backups && delete_mode && !delete_excluded && !am_server) {
 		snprintf(backup_dir_buf, sizeof backup_dir_buf,
 			"P *%s", backup_suffix);
@@ -2364,11 +2381,11 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 		if (whole_file > 0) {
 			snprintf(err_buf, sizeof err_buf,
 				 "--append cannot be used with --whole-file\n");
-			return 0;
+			goto cleanup;
 		}
 		if (refused_inplace) {
 			create_refuse_error(refused_inplace);
-			return 0;
+			goto cleanup;
 		}
 		inplace = 1;
 	}
@@ -2376,7 +2393,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 	if (write_devices) {
 		if (refused_inplace) {
 			create_refuse_error(refused_inplace);
-			return 0;
+			goto cleanup;
 		}
 		inplace = 1;
 	}
@@ -2391,13 +2408,13 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 				 "--%s cannot be used with --%s\n",
 				 append_mode ? "append" : "inplace",
 				 delay_updates ? "delay-updates" : "partial-dir");
-			return 0;
+			goto cleanup;
 		}
 		/* --inplace implies --partial for refusal purposes, but we
 		 * clear the keep_partial flag for internal logic purposes. */
 		if (refused_partial) {
 			create_refuse_error(refused_partial);
-			return 0;
+			goto cleanup;
 		}
 		keep_partial = 0;
 #else
@@ -2405,7 +2422,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			 "--%s is not supported on this %s\n",
 			 append_mode ? "append" : "inplace",
 			 am_server ? "server" : "client");
-		return 0;
+		goto cleanup;
 #endif
 	} else {
 		if (keep_partial && !partial_dir && !am_server) {
@@ -2419,7 +2436,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 				partial_dir = NULL;
 			if (!partial_dir && refused_partial) {
 				create_refuse_error(refused_partial);
-				return 0;
+				goto cleanup;
 			}
 			keep_partial = 1;
 		}
@@ -2440,14 +2457,14 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			if (am_server) {
 				snprintf(err_buf, sizeof err_buf,
 					"The --files-from sent to the server cannot specify a host.\n");
-				return 0;
+				goto cleanup;
 			}
 			files_from = p;
 			filesfrom_host = h;
 			if (strcmp(files_from, "-") == 0) {
 				snprintf(err_buf, sizeof err_buf,
 					"Invalid --files-from remote filename\n");
-				return 0;
+				goto cleanup;
 			}
 		} else {
 			if (sanitize_paths)
@@ -2466,7 +2483,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 				snprintf(err_buf, sizeof err_buf,
 					"failed to open files-from file %s: %s\n",
 					files_from, strerror(errno));
-				return 0;
+				goto cleanup;
 			}
 		}
 	}
@@ -2483,6 +2500,9 @@ int parse_arguments(int *argc_p, const char ***argv_p)
   options_rejected:
 	snprintf(err_buf, sizeof err_buf,
 		"Your options have been rejected by the server.\n");
+  cleanup:
+	if (pc)
+		poptFreeContext(pc);
 	return 0;
 }