From a412f8afc603378a789ce4e8d18bc6d9c4ca3d13 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 5 Jan 2018 14:33:25 -0800 Subject: [PATCH 2/3] =?UTF-8?q?mv:=20fewer=20syscalls=20for=20=E2=80=98mv?= =?UTF-8?q?=20a=20b=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This builds on a previous patch for mv atomicity (Bug#29961). It merely improves performance; it does not fix bugs. * src/copy.h (struct cp_options): New members last_file, rename_errno. * src/copy.c (copy_internal): Support new rename_errno member for the copy options. Avoid calling stat when new members suggest it’s not needed. (cp_options_default): Initialize new members. * src/mv.c: Include renameat2.h. (main): With two arguments, first call ‘renamat2 (AT_FDCWD, "a", AT_FDCWD, "b", RENAME_NOREPLACE)’. Use its results to skip remaining processing if possible; for example, if it succeeds there is no need to stat either "a" or "b". Also, set x.last_file when it is the last file to rename. --- src/copy.c | 67 +++++++++++++++++++++++++++++++------------------------------- src/copy.h | 9 +++++++++ src/mv.c | 22 +++++++++++++++++---- 3 files changed, 61 insertions(+), 37 deletions(-) diff --git a/src/copy.c b/src/copy.c index a1dce8e87..e050d4199 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1854,7 +1854,7 @@ copy_internal (char const *src_name, char const *dst_name, { struct stat src_sb; struct stat dst_sb; - mode_t src_mode; + mode_t src_mode IF_LINT ( = 0); mode_t dst_mode IF_LINT ( = 0); mode_t dst_mode_bits; mode_t omitted_permissions; @@ -1866,33 +1866,48 @@ copy_internal (char const *src_name, char const *dst_name, bool dest_is_symlink = false; bool have_dst_lstat = false; - if (x->move_mode && rename_succeeded) - *rename_succeeded = false; - *copy_into_self = false; - if (XSTAT (x, src_name, &src_sb) != 0) + int rename_errno = x->rename_errno; + if (x->move_mode) { - error (0, errno, _("cannot stat %s"), quoteaf (src_name)); - return false; + if (rename_errno < 0) + rename_errno = (renameat2 (AT_FDCWD, src_name, AT_FDCWD, dst_name, + RENAME_NOREPLACE) + ? errno : 0); + new_dst = rename_errno == 0; + if (rename_succeeded) + *rename_succeeded = new_dst; } - src_mode = src_sb.st_mode; - - if (S_ISDIR (src_mode) && !x->recursive) + if (rename_errno == 0 + ? !x->last_file + : rename_errno != EEXIST || x->interactive != I_ALWAYS_NO) { - error (0, 0, ! x->install_mode /* cp */ - ? _("-r not specified; omitting directory %s") - : _("omitting directory %s"), - quoteaf (src_name)); - return false; + char const *name = rename_errno == 0 ? dst_name : src_name; + if (XSTAT (x, name, &src_sb) != 0) + { + error (0, errno, _("cannot stat %s"), quoteaf (name)); + return false; + } + + src_mode = src_sb.st_mode; + + if (S_ISDIR (src_mode) && !x->recursive) + { + error (0, 0, ! x->install_mode /* cp */ + ? _("-r not specified; omitting directory %s") + : _("omitting directory %s"), + quoteaf (src_name)); + return false; + } } /* Detect the case in which the same source file appears more than once on the command line and no backup option has been selected. If so, simply warn and don't copy it the second time. This check is enabled only if x->src_info is non-NULL. */ - if (command_line_arg) + if (command_line_arg && x->src_info) { if ( ! S_ISDIR (src_sb.st_mode) && x->backup_type == no_backups @@ -1908,21 +1923,6 @@ copy_internal (char const *src_name, char const *dst_name, bool dereference = should_dereference (x, command_line_arg); - int rename_errno = -1; - if (x->move_mode) - { - if (renameat2 (AT_FDCWD, src_name, AT_FDCWD, dst_name, RENAME_NOREPLACE) - != 0) - rename_errno = errno; - else - { - rename_errno = 0; - new_dst = true; - if (rename_succeeded) - *rename_succeeded = true; - } - } - if (!new_dst) { if (! (rename_errno == EEXIST && x->interactive == I_ALWAYS_NO)) @@ -1970,7 +1970,7 @@ copy_internal (char const *src_name, char const *dst_name, return false; } - if (!S_ISDIR (src_mode) && x->update) + if (x->update && !S_ISDIR (src_mode)) { /* When preserving timestamps (but not moving within a file system), don't worry if the destination timestamp is @@ -2360,7 +2360,7 @@ copy_internal (char const *src_name, char const *dst_name, if (rename_succeeded) *rename_succeeded = true; - if (command_line_arg) + if (command_line_arg && !x->last_file) { /* Record destination dev/ino/name, so that if we are asked to overwrite that file again, we can detect it and fail. */ @@ -2998,6 +2998,7 @@ cp_options_default (struct cp_options *x) #else x->chown_privileges = x->owner_privileges = (geteuid () == ROOT_UID); #endif + x->rename_errno = -1; } /* Return true if it's OK for chown to fail, where errno is diff --git a/src/copy.h b/src/copy.h index dd03385c3..c7fdcffbe 100644 --- a/src/copy.h +++ b/src/copy.h @@ -249,6 +249,15 @@ struct cp_options such a symlink) and returns false. */ bool open_dangling_dest_symlink; + /* If true, this is the last filed to be copied. mv uses this to + avoid some unnecessary work. */ + bool last_file; + + /* Zero if the source has already been renamed to the destination; a + positive errno number if this failed with the given errno; -1 if + no attempt has been made to rename. Always -1, except for mv. */ + int rename_errno; + /* Control creation of COW files. */ enum Reflink_type reflink_mode; diff --git a/src/mv.c b/src/mv.c index 818ca59b6..edc1e733f 100644 --- a/src/mv.c +++ b/src/mv.c @@ -31,6 +31,7 @@ #include "error.h" #include "filenamecat.h" #include "remove.h" +#include "renameat2.h" #include "root-dev-ino.h" #include "priv-set.h" @@ -452,8 +453,15 @@ main (int argc, char **argv) else if (!target_directory) { assert (2 <= n_files); - if (target_directory_operand (file[n_files - 1])) - target_directory = file[--n_files]; + if (n_files == 2) + x.rename_errno = (renameat2 (AT_FDCWD, file[0], AT_FDCWD, file[1], + RENAME_NOREPLACE) + ? errno : 0); + if (x.rename_errno != 0 && target_directory_operand (file[n_files - 1])) + { + x.rename_errno = -1; + target_directory = file[--n_files]; + } else if (2 < n_files) die (EXIT_FAILURE, 0, _("target %s is not a directory"), quoteaf (file[n_files - 1])); @@ -487,10 +495,16 @@ main (int argc, char **argv) ok = true; for (int i = 0; i < n_files; ++i) - ok &= movefile (file[i], target_directory, true, &x); + { + x.last_file = i + 1 == n_files; + ok &= movefile (file[i], target_directory, true, &x); + } } else - ok = movefile (file[0], file[1], false, &x); + { + x.last_file = true; + ok = movefile (file[0], file[1], false, &x); + } return ok ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.14.3