]> git.ipfire.org Git - thirdparty/rsync.git/commitdiff
Improve how negotiated info affects batch files.
authorWayne Davison <wayne@opencoder.net>
Tue, 26 May 2020 01:03:02 +0000 (18:03 -0700)
committerWayne Davison <wayne@opencoder.net>
Tue, 26 May 2020 02:19:59 +0000 (19:19 -0700)
batch.c
checksum.c
compat.c
flist.c
io.c
main.c

diff --git a/batch.c b/batch.c
index 77ad4733fc7b4db7be41ae21ff22029cb52d26bd..b60752fc710feac27c16a996e7d747685ec47ebe 100644 (file)
--- a/batch.c
+++ b/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;
 }
index cd84bdb7de5af20ea1e1710da39fd9cb37585706..5576ede95ed549c6111b0adca26ef90fa97e2651 100644 (file)
@@ -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);
        }
 }
 
index 4bef882069a013f8bc751eed316e60aa0c52da05..ccf9c3d75cfd16dcdaa0ef7c535e38b54cfd8806 100644 (file)
--- a/compat.c
+++ b/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);
        }
 
-       init_flist();
-}
+       parse_checksum_choice(1); /* Sets checksum_type & xfersum_type */
+       parse_compress_choice(1); /* Sets do_compression */
 
-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 (write_batch && !am_server)
+               write_batch_shell_file();
 
-       if (valid_compressions.negotiated_name)
-               write_vstring(batch_fd, valid_compressions.negotiated_name, strlen(valid_compressions.negotiated_name));
+       init_flist();
 }
diff --git a/flist.c b/flist.c
index 7e08fd4057a437082636ee2566eda24c8c32d173..82a5d6a7e97643f26137d9eca52b3c7d7ded39f7 100644 (file)
--- a/flist.c
+++ b/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;
diff --git a/io.c b/io.c
index ecab460a93fde22aace4e7ded62e372407396c9e..01d6152f40d224c12f40fc79dd4e46a5e4ed6ca2 100644 (file)
--- a/io.c
+++ b/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)
diff --git a/main.c b/main.c
index 2e2094d26bcafb8e31a55bc81693168b0bec6b39..ed1a210fe32a3ffd82444bd93da745d062be0bbb 100644 (file)
--- a/main.c
+++ b/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