Re: bug fix: mv and 'cp -r' no longer fail when ...

[Jim Meyering]

Bruno Haible <address@hidden> wrote:
> Jim Meyering wrote:
...
> then shouldn't the first test be this?
>
>      if (len <= FILE_SYSTEM_PREFIX_LEN (file) + 1)
>        return false;

Yes, that is better: more efficient on non-POSIX systems.
Thanks.

>> +  {
>> +    /* Fail now, unless SRC is a directory.  */
>> +    struct stat sb;
>> +    if (lstat (src, &sb) != 0 || ! S_ISDIR (sb.st_mode))
>> +      return ret_val;
>> +  }
>> +
>> +  /* Don't call rename again if there are no trailing slashes.  */
>> +  d_len = strlen (dst);
>> +  if ( ! has_trailing_slash (dst, d_len))
>> +    return ret_val;
>
> How about reversing the order of these two tests? Doing an strlen
> is much cheaper than a system call. If you do the trailing slash first,
> you save a system call in many cases.

Same here.  I'll change it on principle, bearing in mind that this
will affect only those lucky few who use NetBSD and invoke mv to rename
a nonexistent SRC to a new name specified without a trailing slash.
So, mv will issue one fewer system call before it reports the failure.

2006-09-15  Jim Meyering  <address@hidden>

        * rename-dest-slash.c (has_trailing_slash): Use
        FILE_SYSTEM_PREFIX_LEN, for non-POSIX systems.
        (rpl_rename_dest_slash): Perform the cheaper trailing slash
        test before testing whether SRC is a directory.
        Suggestions from Bruno Haible.

Index: lib/rename-dest-slash.c
===================================================================
RCS file: /sources/gnulib/gnulib/lib/rename-dest-slash.c,v
retrieving revision 1.1
diff -u -r1.1 rename-dest-slash.c
--- lib/rename-dest-slash.c     15 Sep 2006 13:49:12 -0000      1.1
+++ lib/rename-dest-slash.c     15 Sep 2006 18:33:29 -0000
@@ -42,7 +42,7 @@
 has_trailing_slash (char const *file, size_t len)
 {
   /* Don't count "/" as having a trailing slash.  */
-  if (len <= 1)
+  if (len <= FILE_SYSTEM_PREFIX_LEN (file) + 1)
     return false;

   char last = file[len - 1];
@@ -64,6 +64,11 @@
   if (ret_val == 0 || errno != ENOENT)
     return ret_val;

+  /* Don't call rename again if there are no trailing slashes.  */
+  d_len = strlen (dst);
+  if ( ! has_trailing_slash (dst, d_len))
+    return ret_val;
+
   {
     /* Fail now, unless SRC is a directory.  */
     struct stat sb;
@@ -71,11 +76,6 @@
       return ret_val;
   }

-  /* Don't call rename again if there are no trailing slashes.  */
-  d_len = strlen (dst);
-  if ( ! has_trailing_slash (dst, d_len))
-    return ret_val;
-
   {
     char *dst_temp;
     dst_temp = xmemdup (dst, d_len + 1);