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: [EMAIL PROTECTED] Remember Cernan and Schmitt
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
-- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html