File name too long

[jw schultz]

On Wed, Mar 12, 2003 at 10:27:38AM +0100, Paul Slootman wrote:
> On Tue 11 Mar 2003, jw schultz wrote:
> > 
> > I threw this out there because it looked like we were
> > heading towards a rewrite of the whole function and i wanted
> > to rethink it instead of just reworking parts of it.  If a
> > concensus is that this is the way to go i'm all for that.
> > If the pointer arithmatic is too offensive that's fine too.
> 
> A rewrite is fine; and personally I like pointer arithmetic :-)
> 
> So this is a final draft IMHO:
> 
> 
> /**
>  * get_tmpname() - create a tmp filename for a given filename
>  *   If a tmpdir is defined, use that as the directory to put it in.
>  *   Otherwise, the tmp filename is in the same directory as the given name.
>  *   Note that there may be no directory at all in the given name!
>  *
>  *   The tmp filename is basically the given filename with a dot prepended,
>  *   and .XXXXXX appended (for mkstemp() to put its unique gunk in).
>  *   Take care to not exceed either the MAXPATHLEN or NAME_MAX, esp. the
>  *   last, as the basename basically becomes 8 chars longer. In that case,
>  *   the original name is shortened sufficiently to make it all fit.
>  *
>  *   Of course, there's no real reason for the tmp name to look like the
>  *   original, except to satisfy us humans. As long as it's unique, rsync
>  *   will work.
>  */
> static int get_tmpname(char *fnametmp, char *fname)
> {
> 	char	*f;
> 	int	length = 0;
> 	int	maxname;
> 
> 	if (tmpdir) {
> 		strlcpy(fnametmp, tmpdir, MAXPATHLEN - 2);
> 		length = strlen(fnametmp);
> 		strcpy(fnametmp + length++, "/");
> 	}
> 
> 	if ((f = strrchr(fname, '/'))) {
> 		++f;
> 		if (!tmpdir) {
> 			/* copy up to and including the slash */
> 			length = f - fname;
> 			strlcpy(fnametmp, fname, length);
> 		}
> 	}
> 	else {
> 		f = fname;
> 	}
> 	strcpy(fnametmp + length++, ".");
> 
> 	maxname = MIN(MAXPATHLEN - 7 - length, NAME_MAX - 8);
> 
> 	if (maxname < 1) {
> 		rprintf(FERROR, "temporary filename too long: %s\n", fname);
> 		return 0;
> 	}
> 
> 	strlcpy(fnametmp + length, f, maxname);
> 	strcat(fnametmp + length, ".XXXXXX");
> 
> 	return 1;
> }

Not quite final.  I really didn't like calling strcpy for
single chars so i fixed that.  Also strlcpy counts the null
in the length so a small adjustment had to be made.

Here it is in as a patch to cvs.  Ready for commit since i
don't have that privilege yet.  It builds and passes the
testsuite.


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

		Remember Cernan and Schmitt
-------------- next part --------------
Index: receiver.c
===================================================================
RCS file: /cvsroot/rsync/receiver.c,v
retrieving revision 1.41
diff -b -u -r1.41 receiver.c
--- receiver.c	20 Jan 2003 23:32:17 -0000	1.41
+++ receiver.c	12 Mar 2003 11:08:36 -0000
@@ -164,40 +164,63 @@
 }
 
 
+/*
+ * get_tmpname() - create a tmp filename for a given filename
+ *
+ *   If a tmpdir is defined, use that as the directory to
+ *   put it in.  Otherwise, the tmp filename is in the same
+ *   directory as the given name.  Note that there may be no
+ *   directory at all in the given name!
+ *      
+ *   The tmp filename is basically the given filename with a
+ *   dot prepended, and .XXXXXX appended (for mkstemp() to
+ *   put its unique gunk in).  Take care to not exceed
+ *   either the MAXPATHLEN or NAME_MAX, esp. the last, as
+ *   the basename basically becomes 8 chars longer. In that
+ *   case, the original name is shortened sufficiently to
+ *   make it all fit.
+ *      
+ *   Of course, there's no real reason for the tmp name to
+ *   look like the original, except to satisfy us humans.
+ *   As long as it's unique, rsync will work.
+ */
+
 static int get_tmpname(char *fnametmp, char *fname)
 {
 	char *f;
+	int     length = 0;
+	int	maxname;
 
-	/* open tmp file */
 	if (tmpdir) {
-		f = strrchr(fname,'/');
-		if (f == NULL) 
-			f = fname;
-		else 
-			f++;
-		if (strlen(tmpdir)+strlen(f)+10 > MAXPATHLEN) {
-			rprintf(FERROR,"filename too long\n");
-			return 0;
+		strlcpy(fnametmp, tmpdir, MAXPATHLEN - 2);
+		length = strlen(fnametmp);
+		fnametmp[length++] = '/';
+		fnametmp[length] = '\0';	/* always NULL terminated */
 		}
-		snprintf(fnametmp,MAXPATHLEN, "%s/.%s.XXXXXX",tmpdir,f);
-		return 1;
+
+	if ((f = strrchr(fname, '/'))) {	/* extra () for gcc */
+		++f;
+		if (!tmpdir) {
+			length = f - fname;
+			strlcpy(fnametmp, fname, length + 1);
+		}		/* copy up to and including the slash */
+	} else {
+		f = fname;
 	} 
+	fnametmp[length++] = '.';
+	fnametmp[length] = '\0';		/* always NULL terminated */
 
-	f = strrchr(fname,'/');
+	maxname = MIN(MAXPATHLEN - 7 - length, NAME_MAX - 8);
 
-	if (strlen(fname)+9 > MAXPATHLEN) {
-		rprintf(FERROR,"filename too long\n");
+	if (maxname < 1)
+	{
+		rprintf(FERROR, "temporary filename too long: %s\n", fname);
+		fnametmp[0] = '\0';
 		return 0;
 	}
 
-	if (f) {
-		*f = 0;
-		snprintf(fnametmp,MAXPATHLEN,"%s/.%s.XXXXXX",
-			 fname,f+1);
-		*f = '/';
-	} else {
-		snprintf(fnametmp,MAXPATHLEN,".%s.XXXXXX",fname);
-	}
+	strlcpy(fnametmp + length, f, maxname); 
+	strcat(fnametmp + length, ".XXXXXX");
 
 	return 1;
 }
@@ -435,7 +458,7 @@
 			fd2 = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS);
 		}
 		if (fd2 == -1) {
-			rprintf(FERROR,"mkstemp %s failed: %s\n",fnametmp,strerror(errno));
+			rprintf(FERROR,"mkstemp s %s failed: %s\n", fname, strerror(errno));
 			receive_data(f_in,buf,-1,NULL,file->length);
 			if (buf) unmap_file(buf);
 			if (fd1 != -1) close(fd1);
Index: rsync.h
===================================================================
RCS file: /cvsroot/rsync/rsync.h,v
retrieving revision 1.137
diff -b -u -r1.137 rsync.h
--- rsync.h	18 Feb 2003 18:07:36 -0000	1.137
+++ rsync.h	12 Mar 2003 11:08:38 -0000
@@ -327,6 +327,10 @@
 #define MAXPATHLEN 1024
 #endif
 
+#ifndef NAME_MAX
+#define NAME_MAX 255
+#endif
+
 #ifndef INADDR_NONE
 #define INADDR_NONE 0xffffffff
 #endif