Inadequate error checking in rsync 2.5.5

[jw schultz]

On Thu, Aug 14, 2003 at 03:08:22PM -0700, jw schultz wrote:
> On Thu, Aug 14, 2003 at 12:41:19PM -0700, David Norwood wrote:
> [reformatted because someone doesn't know how to compose email]
> > I'm using rsync to mirror files from a Windows XP machine
> > mounted via smbfs.  Apparently I have something configured
> > wrong as I get a "permisson denied" error accessing some
> > of the files on the smbfs mount with cp, od, etc.
> > However, rsync produces no error messages on these files.
> > It happily creates files in the target directory that are
> > the right size, but filled with null bytes.  
> > 
> > This is repeatable on my system, until I figure out the
> > smbfs permisson problem.  I'm running RedHat 9.  I'm not
> > subscribed to this list, so please cc me on any replies.
> 
> If you are going to report problems the least you could do
> is upgrade to a current version.  Unfortunately, i don't
> expect that will fix this.
> 
> This looks like a situation similar to that which NFS can
> have where open succeeds but read fails.  Unfortunately
> there isn't really a good way to detect this until we get
> into it and then it is indistinguishable from a file
> being truncated out from under us.
> 
> It is down in map_ptr() where we don't have knowledge of the
> file name and can't really report errors to the caller.
> 
> 	if ((nread=read(map->fd,map->p + read_offset,read_size)) != read_size) {
> 		if (nread < 0) nread = 0;
> 		/* the best we can do is zero the buffer - the file
> 		   has changed mid transfer! */
> 		memset(map->p+read_offset+nread, 0, read_size - nread);
> 	}

I've had a chance to think on it.  Attached is a patch that
allows unmap_file() to report the first read error that
map_ptr found.  The behaviour is the same.  I doubt this will
apply against anything but CVS HEAD as of now.

This should probably use FERROR instead of FINFO so that a
partial transfer is reported.

Any thoughts guys?

-- 
________________________________________________________________
	J.W. Schultz            Pegasystems Technologies
	email address:		jw at pegasys.ws

		Remember Cernan and Schmitt
-------------- next part --------------
? receiver.diff
Index: fileio.c
===================================================================
RCS file: /data/cvs/rsync/fileio.c,v
retrieving revision 1.6
diff -u -r1.6 fileio.c
--- fileio.c	22 May 2003 23:24:44 -0000	1.6
+++ fileio.c	15 Aug 2003 01:11:14 -0000
@@ -115,6 +115,7 @@
 	map->p_offset = 0;
 	map->p_fd_offset = 0;
 	map->p_len = 0;
+	map->status = 0;
 
 	return map;
 }
@@ -191,7 +192,11 @@
 		}
 
 		if ((nread=read(map->fd,map->p + read_offset,read_size)) != read_size) {
-			if (nread < 0) nread = 0;
+			if (nread < 0) {
+				nread = 0;
+				if (!map->status)
+					map->status = errno;
+			}
 			/* the best we can do is zero the buffer - the file
 			   has changed mid transfer! */
 			memset(map->p+read_offset+nread, 0, read_size - nread);
@@ -206,13 +211,18 @@
 }
 
 
-void unmap_file(struct map_struct *map)
+int unmap_file(struct map_struct *map)
 {
+	int	ret;
+
 	if (map->p) {
 		free(map->p);
 		map->p = NULL;
 	}
+	ret = map->status;
 	memset(map, 0, sizeof(*map));
 	free(map);
+
+	return ret;
 }
 
Index: proto.h
===================================================================
RCS file: /data/cvs/rsync/proto.h,v
retrieving revision 1.158
diff -u -r1.158 proto.h
--- proto.h	1 Jul 2003 21:45:25 -0000	1.158
+++ proto.h	15 Aug 2003 01:11:14 -0000
@@ -71,7 +71,7 @@
 int write_file(int f,char *buf,size_t len);
 struct map_struct *map_file(int fd,OFF_T len);
 char *map_ptr(struct map_struct *map,OFF_T offset,int len);
-void unmap_file(struct map_struct *map);
+int unmap_file(struct map_struct *map);
 void show_flist_stats(void);
 int readlink_stat(const char *path, STRUCT_STAT * buffer, char *linkbuf);
 int link_stat(const char *path, STRUCT_STAT * buffer);
@@ -191,7 +191,7 @@
 void sig_int(void);
 void finish_transfer(char *fname, char *fnametmp, struct file_struct *file);
 void read_sum_head(int f, struct sum_struct *sum);
-void send_files(struct file_list *flist,int f_out,int f_in);
+void send_files(struct file_list *flist, int f_out, int f_in);
 int try_bind_local(int s,
 		   int ai_family, int ai_socktype,
 		   const char *bind_address);
Index: rsync.h
===================================================================
RCS file: /data/cvs/rsync/rsync.h,v
retrieving revision 1.152
diff -u -r1.152 rsync.h
--- rsync.h	30 Jul 2003 06:12:31 -0000	1.152
+++ rsync.h	15 Aug 2003 01:11:15 -0000
@@ -420,9 +420,17 @@
 };
 
 struct map_struct {
-	char *p;
-	int fd,p_size,p_len;
-	OFF_T file_size, p_offset, p_fd_offset;
+	char *p;		/* Window pointer			*/
+	int fd;			/* File Descriptor			*/
+	int p_size;		/* Window size at allocation		*/
+	int p_len;		/* Window size after fill		*/
+				/*    p_size and p_len could be
+				 *    consolodated by using a local
+				 *    variable in map_ptr()		*/
+	int status;		/* first errno from read errors		*/
+	OFF_T file_size;	/* File size (from stat)		*/
+	OFF_T p_offset;		/* Window start				*/
+	OFF_T p_fd_offset;	/* offset of cursor in fd ala lseek	*/
 };
 
 #define MATCHFLG_WILD		0x0001 /* pattern has '*', '[', and/or '?' */
Index: sender.c
===================================================================
RCS file: /data/cvs/rsync/sender.c,v
retrieving revision 1.21
diff -u -r1.21 sender.c
--- sender.c	15 Aug 2003 00:57:27 -0000	1.21
+++ sender.c	15 Aug 2003 01:11:15 -0000
@@ -279,7 +279,14 @@
 		}
 
 		if (!read_batch) { /* dw */
-			if (buf) unmap_file(buf);
+			if (buf) {
+				j = unmap_file(buf);
+				if (j)
+					rprintf(FINFO,
+					    "read errors mapping %s: %s\n",
+					    fname,
+					    strerror(j));
+			}
 			close(fd);
 		}