Improve how negotiated info affects batch files.

batch.c

@@ -37,14 +37,21 @@ extern int always_checksum;
 extern int do_compression;
 extern int inplace;
 extern int append_mode;
+extern int write_batch;
 extern int protocol_version;
+extern int raw_argc, cooked_argc;
+extern char **raw_argv, **cooked_argv;
 extern char *batch_name;
+extern const char *checksum_choice;
+extern const char *compress_choice;
 #ifdef ICONV_OPTION
 extern char *iconv_opt;
 #endif
 
 extern filter_rule_list filter_list;
 
+int batch_fd = -1;
+int batch_sh_fd = -1;
 int batch_stream_flags;
 
 static int tweaked_append;
@@ -156,37 +163,45 @@ void check_batch_flags(void)
 		append_mode = 2;
 }
 
-static int write_arg(int fd, char *arg)
+static int write_arg(const char *arg)
 {
-	char *x, *s;
-	int len, ret = 0;
+	const char *x, *s;
+	int len, err = 0;
 
 	if (*arg == '-' && (x = strchr(arg, '=')) != NULL) {
-		if (write(fd, arg, x - arg + 1) != x - arg + 1)
-			ret = -1;
+		err |= write(batch_sh_fd, arg, x - arg + 1) != x - arg + 1;
 		arg += x - arg + 1;
 	}
 
 	if (strpbrk(arg, " \"'&;|[]()$#!*?^\\") != NULL) {
-		if (write(fd, "'", 1) != 1)
-			ret = -1;
+		err |= write(batch_sh_fd, "'", 1) != 1;
 		for (s = arg; (x = strchr(s, '\'')) != NULL; s = x + 1) {
-			if (write(fd, s, x - s + 1) != x - s + 1
-			 || write(fd, "'", 1) != 1)
-				ret = -1;
+			err |= write(batch_sh_fd, s, x - s + 1) != x - s + 1;
+			err |= write(batch_sh_fd, "'", 1) != 1;
 		}
 		len = strlen(s);
-		if (write(fd, s, len) != len
-		 || write(fd, "'", 1) != 1)
-			ret = -1;
-		return ret;
+		err |= write(batch_sh_fd, s, len) != len;
+		err |= write(batch_sh_fd, "'", 1) != 1;
+		return err;
 	}
 
 	len = strlen(arg);
-	if (write(fd, arg, len) != len)
-		ret = -1;
+	err |= write(batch_sh_fd, arg, len) != len;
 
-	return ret;
+	return err;
+}
+
+/* Writes out a space and then an option (or other string) with an optional "=" + arg suffix. */
+static int write_opt(const char *opt, const char *arg)
+{
+	int len = strlen(opt);
+	int err = write(batch_sh_fd, " ", 1) != 1;
+	err = write(batch_sh_fd, opt, len) != len ? 1 : 0; 
+	if (arg) {
+		err |= write(batch_sh_fd, "=", 1) != 1;
+		err |= write_arg(arg);
+	}
+	return err;
 }
 
 static void write_filter_rules(int fd)
@@ -208,42 +223,63 @@ static void write_filter_rules(int fd)
 	write_sbuf(fd, "#E#");
 }
 
+/* This sets batch_fd and (for --write-batch) batch_sh_fd. */
+void open_batch_files(void)
+{
+	if (write_batch) {
+		char filename[MAXPATHLEN];
+
+		stringjoin(filename, sizeof filename, batch_name, ".sh", NULL);
+
+		batch_sh_fd = do_open(filename, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR | S_IXUSR);
+		if (batch_sh_fd < 0) {
+			rsyserr(FERROR, errno, "Batch file %s open error", full_fname(filename));
+			exit_cleanup(RERR_FILESELECT);
+		}
+
+		batch_fd = do_open(batch_name, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
+	} else if (strcmp(batch_name, "-") == 0)
+		batch_fd = STDIN_FILENO;
+	else
+		batch_fd = do_open(batch_name, O_RDONLY, S_IRUSR | S_IWUSR);
+
+	if (batch_fd < 0) {
+		rsyserr(FERROR, errno, "Batch file %s open error", full_fname(batch_name));
+		exit_cleanup(RERR_FILEIO);
+	}
+}
+
 /* This routine tries to write out an equivalent --read-batch command
  * given the user's --write-batch args.  However, it doesn't really
  * understand most of the options, so it uses some overly simple
  * heuristics to munge the command line into something that will
  * (hopefully) work. */
-void write_batch_shell_file(int argc, char *argv[], int file_argc, char *file_argv[])
+void write_batch_shell_file(void)
 {
-	int fd, i, len, err = 0;
-	char *p, *p2, filename[MAXPATHLEN];
-
-	stringjoin(filename, sizeof filename,
-		   batch_name, ".sh", NULL);
-	fd = do_open(filename, O_WRONLY | O_CREAT | O_TRUNC,
-		     S_IRUSR | S_IWUSR | S_IXUSR);
-	if (fd < 0) {
-		rsyserr(FERROR, errno, "Batch file %s open error",
-			filename);
-		exit_cleanup(RERR_FILESELECT);
-	}
+	int i, len, err = 0;
+	char *p, *p2;
 
 	/* Write argvs info to BATCH.sh file */
-	if (write_arg(fd, argv[0]) < 0)
-		err = 1;
+	err |= write_arg(raw_argv[0]);
 	if (filter_list.head) {
 		if (protocol_version >= 29)
-			write_sbuf(fd, " --filter=._-");
+			err |= write_opt("--filter", "._-");
 		else
-			write_sbuf(fd, " --exclude-from=-");
+			err |= write_opt("--exclude-from", "-");
 	}
-	for (i = 1; i < argc; i++) {
-		p = argv[i];
-		if (file_argc && p == file_argv[0]) {
-			if (file_argc > 1) {
-				file_argv++;
-				file_argc--;
-			}
+
+	/* We need to make sure that any protocol-based or negotiated choices get accurately
+	 * reflected in the options we save AND that we avoid any need for --read-batch to
+	 * do a string-based negotation (since we don't write them into the file). */
+	if (do_compression)
+		err |= write_opt("--compress-choice", compress_choice);
+	err |= write_opt("--checksum-choice", checksum_choice);
+
+	for (i = 1; i < raw_argc; i++) {
+		p = raw_argv[i];
+		if (cooked_argc && p[0] == cooked_argv[0][0] && strcmp(p, cooked_argv[0]) == 0) {
+			cooked_argv++;
+			cooked_argc--;
 			continue;
 		}
 		if (strncmp(p, "--files-from", 12) == 0
@@ -258,33 +294,24 @@ void write_batch_shell_file(int argc, char *argv[], int file_argc, char *file_ar
 			i++;
 			continue;
 		}
-		if (write(fd, " ", 1) != 1)
-			err = 1;
 		if (strncmp(p, "--write-batch", len = 13) == 0
-		 || strncmp(p, "--only-write-batch", len = 18) == 0) {
-			if (write(fd, "--read-batch", 12) != 12)
-				err = 1;
-			if (p[len] == '=') {
-				if (write(fd, "=", 1) != 1
-				 || write_arg(fd, p + len + 1) < 0)
-					err = 1;
-			}
-		} else {
-			if (write_arg(fd, p) < 0)
-				err = 1;
+		 || strncmp(p, "--only-write-batch", len = 18) == 0)
+			err |= write_opt("--read-batch", p[len] == '=' ? p + len + 1 : NULL);
+		else {
+			err |= write(batch_sh_fd, " ", 1) != 1;
+			err |= write_arg(p);
 		}
 	}
-	if (!(p = check_for_hostspec(file_argv[file_argc - 1], &p2, &i)))
-		p = file_argv[file_argc - 1];
-	if (write(fd, " ${1:-", 6) != 6
-	 || write_arg(fd, p) < 0)
-		err = 1;
-	write_byte(fd, '}');
+	if (!(p = check_for_hostspec(cooked_argv[cooked_argc - 1], &p2, &i)))
+		p = cooked_argv[cooked_argc - 1];
+	err |= write_opt("${1:-", NULL);
+	err |= write_arg(p);
+	err |= write(batch_sh_fd, "}", 1) != 1;
 	if (filter_list.head)
-		write_filter_rules(fd);
-	if (write(fd, "\n", 1) != 1 || close(fd) < 0 || err) {
-		rsyserr(FERROR, errno, "Batch file %s write error",
-			filename);
+		write_filter_rules(batch_sh_fd);
+	if (write(batch_sh_fd, "\n", 1) != 1 || close(batch_sh_fd) < 0 || err) {
+		rsyserr(FERROR, errno, "Batch file %s.sh write error", batch_name);
 		exit_cleanup(RERR_FILEIO);
 	}
+	batch_sh_fd = -1;
 }

checksum.c

@@ -40,7 +40,7 @@ extern int whole_file;
 extern int checksum_seed;
 extern int protocol_version;
 extern int proper_seed_order;
-extern char *checksum_choice;
+extern const char *checksum_choice;
 
 #define CSUM_NONE 0
 #define CSUM_MD4_ARCHAIC 1
@@ -123,18 +123,17 @@ void parse_checksum_choice(int final_call)
 	if (xfersum_type == CSUM_NONE)
 		whole_file = 1;
 
+	/* Snag the checksum name for both write_batch's option output & the following debug output. */
+	if (valid_checksums.negotiated_name)
+		checksum_choice = valid_checksums.negotiated_name;
+	else if (checksum_choice == NULL)
+		checksum_choice = checksum_name(xfersum_type);
+
 	if (final_call && DEBUG_GTE(NSTR, am_server ? 3 : 1)) {
-		const char *c_s = am_server ? "Server" : "Client";
-		if (valid_checksums.negotiated_name)
-			rprintf(FINFO, "%s negotiated checksum: %s\n", c_s, valid_checksums.negotiated_name);
-		else if (xfersum_type == checksum_type) {
-			rprintf(FINFO, "%s %s checksum: %s\n", c_s,
-				checksum_choice ? "chosen" : "protocol-based",
-				checksum_name(xfersum_type));
-		} else {
-			rprintf(FINFO, "%s chosen transfer checksum: %s\n", c_s, checksum_name(xfersum_type));
-			rprintf(FINFO, "%s chosen pre-transfer checksum: %s\n", c_s, checksum_name(checksum_type));
-		}
+		rprintf(FINFO, "%s%s checksum: %s\n",
+			am_server ? "Server" : "Client",
+			valid_checksums.negotiated_name ? " negotiated" : "",
+			checksum_choice);
 	}
 }
 

compat.c

@@ -54,8 +54,8 @@ extern char *partial_dir;
 extern char *dest_option;
 extern char *files_from;
 extern char *filesfrom_host;
-extern char *checksum_choice;
-extern char *compress_choice;
+extern const char *checksum_choice;
+extern const char *compress_choice;
 extern filter_rule_list filter_list;
 extern int need_unsorted_flist;
 #ifdef ICONV_OPTION
@@ -187,17 +187,20 @@ void parse_compress_choice(int final_call)
 	if (do_compression == CPRES_NONE)
 		compress_choice = NULL;
 
+	/* Snag the compression name for both write_batch's option output & the following debug output. */
+	if (valid_compressions.negotiated_name)
+		compress_choice = valid_compressions.negotiated_name;
+	else if (compress_choice == NULL) {
+		struct name_num_item *nni = get_nni_by_num(&valid_compressions, do_compression);
+		compress_choice = nni ? nni->name : "UNKNOWN";
+	}
+
 	if (final_call && DEBUG_GTE(NSTR, am_server ? 3 : 1)
 	 && (do_compression != CPRES_NONE || do_compression_level != CLVL_NOT_SPECIFIED)) {
-		const char *c_s = am_server ? "Server" : "Client";
-		if (do_compression != CPRES_NONE && valid_compressions.negotiated_name) {
-			rprintf(FINFO, "%s negotiated compress: %s (level %d)\n",
-				c_s, valid_compressions.negotiated_name, do_compression_level);
-		} else {
-			struct name_num_item *nni = get_nni_by_num(&valid_compressions, do_compression);
-			rprintf(FINFO, "%s compress: %s (level %d)\n",
-				c_s, nni ? nni->name : "UNKNOWN", do_compression_level);
-		}
+		rprintf(FINFO, "%s%s compress: %s (level %d)\n",
+			am_server ? "Server" : "Client",
+			valid_compressions.negotiated_name ? " negotiated" : "",
+			compress_choice, do_compression_level);
 	}
 }
 
@@ -422,8 +425,7 @@ static void send_negotiate_str(int f_out, struct name_num_obj *nno, const char *
 	if (local_server) {
 		/* A local server doesn't bother to send/recv the strings, it just constructs
 		 * and parses the same string on both sides. */
-		if (!read_batch)
-			recv_negotiate_str(-1, nno, tmpbuf, len);
+		recv_negotiate_str(-1, nno, tmpbuf, len);
 	} else {
 		/* Each side sends their list of valid names to the other side and then both sides
 		 * pick the first name in the client's list that is also in the server's list. */
@@ -685,14 +687,11 @@ void setup_protocol(int f_out,int f_in)
 		checksum_seed = read_int(f_in);
 	}
 
+	parse_checksum_choice(1); /* Sets checksum_type & xfersum_type */
+	parse_compress_choice(1); /* Sets do_compression */
+
+	if (write_batch && !am_server)
+		write_batch_shell_file();
+
 	init_flist();
 }
-
-void maybe_write_negotiated_strings(int batch_fd)
-{
-	if (valid_checksums.negotiated_name)
-		write_vstring(batch_fd, valid_checksums.negotiated_name, strlen(valid_checksums.negotiated_name));
-
-	if (valid_compressions.negotiated_name)
-		write_vstring(batch_fd, valid_compressions.negotiated_name, strlen(valid_compressions.negotiated_name));
-}

flist.c

@@ -142,8 +142,6 @@ void init_flist(void)
 		rprintf(FINFO, "FILE_STRUCT_LEN=%d, EXTRA_LEN=%d\n",
 			(int)FILE_STRUCT_LEN, (int)EXTRA_LEN);
 	}
-	parse_checksum_choice(1); /* Sets checksum_type & xfersum_type */
-	parse_compress_choice(1); /* Sets do_compression */
 	flist_csum_len = csum_len_for_type(checksum_type, 1);
 
 	show_filelist_progress = INFO_GTE(FLIST, 1) && xfer_dirs && !am_server && !inc_recurse;

io.c

@@ -44,6 +44,7 @@ extern int am_generator;
 extern int msgs2stderr;
 extern int inc_recurse;
 extern int io_error;
+extern int batch_fd;
 extern int eol_nulls;
 extern int flist_eof;
 extern int file_total;
@@ -67,7 +68,6 @@ extern iconv_t ic_send, ic_recv;
 
 int csum_length = SHORT_SUM_LENGTH; /* initial value */
 int allowed_lull = 0;
-int batch_fd = -1;
 int msgdone_cnt = 0;
 int forward_flist_data = 0;
 BOOL flist_receiving_enabled = False;
@@ -2369,7 +2369,6 @@ void start_write_batch(int fd)
 	write_int(batch_fd, protocol_version);
 	if (protocol_version >= 30)
 		write_varint(batch_fd, compat_flags);
-	maybe_write_negotiated_strings(batch_fd);
 	write_int(batch_fd, checksum_seed);
 
 	if (am_sender)

main.c

@@ -107,6 +107,8 @@ int daemon_over_rsh = 0;
 mode_t orig_umask = 0;
 int batch_gen_fd = -1;
 int sender_keeps_checksum = 0;
+int raw_argc, cooked_argc;
+char **raw_argv, **cooked_argv;
 
 /* There's probably never more than at most 2 outstanding child processes,
  * but set it higher, just in case. */
@@ -1648,8 +1650,10 @@ static void rsync_panic_handler(UNUSED(int whatsig))
 int main(int argc,char *argv[])
 {
 	int ret;
-	int orig_argc = argc;
-	char **orig_argv = argv;
+
+	raw_argc = argc;
+	raw_argv = argv;
+
 #ifdef HAVE_SIGACTION
 # ifdef HAVE_SIGPROCMASK
 	sigset_t sigmask;
@@ -1703,6 +1707,8 @@ int main(int argc,char *argv[])
 		option_error();
 		exit_cleanup(RERR_SYNTAX);
 	}
+	cooked_argc = argc;
+	cooked_argv = argv;
 
 	SIGACTMASK(SIGINT, sig_int);
 	SIGACTMASK(SIGHUP, sig_int);
@@ -1725,21 +1731,7 @@ int main(int argc,char *argv[])
 	change_dir(NULL, CD_NORMAL);
 
 	if ((write_batch || read_batch) && !am_server) {
-		if (write_batch)
-			write_batch_shell_file(orig_argc, orig_argv, argc, argv);
-
-		if (read_batch && strcmp(batch_name, "-") == 0)
-			batch_fd = STDIN_FILENO;
-		else {
-			batch_fd = do_open(batch_name,
-				   write_batch ? O_WRONLY | O_CREAT | O_TRUNC
-				   : O_RDONLY, S_IRUSR | S_IWUSR);
-		}
-		if (batch_fd < 0) {
-			rsyserr(FERROR, errno, "Batch file %s open error",
-				full_fname(batch_name));
-			exit_cleanup(RERR_FILEIO);
-		}
+		open_batch_files(); /* sets batch_fd */
 		if (read_batch)
 			read_stream_flags(batch_fd);
 		else