2.2.5pre1: rename design flaw?

[jd at epcnet.de]

Hi,

after looking in vfs-wrap.c i saw in function vfswrap_rename the following:

 	result = rename(oldname, newname);
 	if (errno == EXDEV) {
 		/* Rename across filesystems needed. */
		result = copy_reg(oldname, newname);
 	}

There is no rename across filesystems for directorys. rename fails with EXDEV. 
Is this ok? I think no and I added a function copy_dir which moves a complete dirtree
 from one device to another.

I added the diff of vfs-wrap.c to this email.

Greetings

    Jochen Dolze


--- vfs-wrap.c.orig	Tue Jun  4 04:11:27 2002
+++ vfs-wrap.c	Wed Jun 12 22:45:04 2002
@@ -1,4 +1,4 @@
-/* 
+/*
    Unix SMB/Netbios implementation.
    Version 1.9.
    Wrap disk only vfs functions to sidestep dodgy compilers.
@@ -83,7 +83,7 @@
 
 	if (result == 0 && !has_dacl) {
 		/*
-		 * We need to do this as the default behavior of POSIX ACLs	
+		 * We need to do this as the default behavior of POSIX ACLs
 		 * is to set the mask to be the requested group permission
 		 * bits, not the group permission bits to be the requested
 		 * group permission bits. This is not what we want, as it will
@@ -195,7 +195,7 @@
 }
 
 /*********************************************************
- For rename across filesystems Patch from Warren Birnbaum 
+ For rename across filesystems Patch from Warren Birnbaum
  <warrenb at hpcvscdp.cv.hp.com>
 **********************************************************/
 
@@ -264,7 +264,12 @@
 	if ((chown(dest, source_stats.st_uid, source_stats.st_gid) == -1) && (errno != EPERM))
 		return -1;
 
-	if (chmod (dest, source_stats.st_mode & 07777))
+	/*
+         * Try to preserve access permissions. Some filesystems (fat, smbfs)
+	 * cannot set all and return EPERM.
+	 */
+
+	if ((chmod (dest, source_stats.st_mode & 07777) == -1) && (errno != EPERM))
 		return -1;
 
 	if (unlink (source) == -1)
@@ -273,6 +278,110 @@
 	return 0;
 }
 
+/*********************************************************
+ For directory renames across filesystems
+**********************************************************/
+
+static int copy_dir(const char *source, const char *dest) {
+
+        DIR *dir;
+        struct dirent *dirent;
+	SMB_STRUCT_STAT source_stats, src_state;
+        char *src, *dst;
+        int slen, dlen;
+        int s_errno;
+
+	if (sys_lstat (source, &source_stats) == -1)
+		return -1;
+
+	if (S_ISREG (source_stats.st_mode))
+                return copy_reg(source, dest);
+
+        if (!S_ISDIR(source_stats.st_mode))
+                return -1;
+
+        if (mkdir(dest, source_stats.st_mode & 07777) == -1)
+                return -1;
+
+        dir = opendir(source);
+        if (!dir)
+                return -1;
+
+        while (dirent = readdir(dir)) {
+
+                if (dirent->d_name[0]=='.') continue;
+
+                slen = strlen(source)+strlen(dirent->d_name)+10;
+                src = malloc(slen);
+                if (!src) {
+                    s_errno = errno;
+                    closedir(dir);
+                    errno = s_errno;
+                    return -1;
+                }
+                dlen = strlen(dest)+strlen(dirent->d_name)+10;
+                dst = malloc(dlen);
+                if (!dst) {
+                    s_errno = errno;
+                    closedir(dir);
+                    free(src);
+                    errno = s_errno;
+                    return -1;
+                }
+
+                snprintf(src,slen,"%s/%s", source, dirent->d_name);
+                snprintf(dst,dlen,"%s/%s", dest, dirent->d_name);
+
+                if (sys_lstat (src, &src_state) == -1) {
+                    s_errno = errno;
+                    closedir(dir);
+                    free(src);
+                    free(dst);
+                    errno = s_errno;
+                    return -1;
+                }
+
+                if (S_ISREG(src_state.st_mode)) {
+                    if (copy_reg(src, dst) == -1) {
+                       s_errno = errno;
+                       closedir(dir);
+                       free(src);
+                       free(dst);
+                       errno = s_errno;
+                       return -1;
+                    }
+                } else
+                if (S_ISDIR(src_state.st_mode)) {
+                    if (copy_dir(src, dst) == -1) {
+                       s_errno = errno;
+                       closedir(dir);
+                       free(src);
+                       free(dst);
+                       errno = s_errno;
+                       return -1;
+                    }
+                } else {
+                    free(src);
+                    free(dst);
+                    closedir(dir);
+                    errno = EXDEV;
+                    return -1;
+                }
+
+                free(src);
+                free(dst);
+        }
+
+        if (closedir(dir) == -1)
+                return -1;
+
+        if (rmdir(source) == -1)
+                return -1;
+
+        return 0;
+
+}
+
 int vfswrap_rename(connection_struct *conn, const char *oldname, const char *newname)
 {
 	int result;
@@ -281,7 +390,7 @@
 	result = rename(oldname, newname);
 	if (errno == EXDEV) {
 		/* Rename across filesystems needed. */
-		result = copy_reg(oldname, newname);
+		result = copy_dir(oldname, newname);
 	}
 	END_PROFILE(syscall_rename);
 	return result;


--- 
EPCNet GmbH
ISP & Web Design
Bleichstrasse 24
89077 Ulm

Tel.  +49 731 1416 0
Fax  +49 731 1416 120