File name too long

jw schultz jw at pegasys.ws
Tue Mar 11 23:16:41 EST 2003


On Tue, Mar 11, 2003 at 10:47:10AM +0100, Paul Slootman wrote:
> On Tue 11 Mar 2003, jw schultz wrote:
> > 
> > That or char fscratch[MAXPATHLEN];
> > Just don't use malloc.
> 
> How about this:
> 
> 
> static int get_tmpname(char *fnametmp, char *fname)
> {
> 	char	*f;
> 	char	*dir = "";	/* what dir to put the temp file in */
> 	char	namepart[NAME_MAX-10];	/* we never need more than this, if   */
> 					/* the name is longer, we would end   */
> 					/* up having to shorten it anyway     */
> 	if (tmpdir)
> 		dir = tmpdir;
> 
> 	f = strrchr(fname,'/');	/* is there a directory to skip in fname? */
> 	if (f == NULL) {		/* no */
> 		/* the strlcpy takes care of making the name short enough */
> 		/* and null-terminating it at the same time. */
> 		strlcpy(namepart, fname, sizeof(namepart));
> 	}
> 	else {				/* yes */
> 		strlcpy(namepart, f+1, sizeof(namepart));

If we aren't using tmpdir we need to strlcpy(dir, fname, f - fname)
at which point we've just copied all of fname anyway.

> 	}
> 
> 	if (strlen(dir)+strlen(namepart)+1 > MAXPATHLEN) {
> 		/* how often will this happen... the temp dir would have to */
> 		/* a very long name, and hence the fault of the user who    */
> 		/* specified it. Let him fix the problem.                   */
> 		rprintf(FERROR,"filename too long\n");
> 		return 0;
> 	}
> 
> 	/* construct temp name template */
> 	snprintf(fnametmp, MAXPATHLEN, "%s%s.%s.XXXXXX",
>                                         dir, dir[0]?"/":"", namepart);

Please use whitespace in the ?: 

> 
> 	return 1;
> }
> 
> 
> Even shorter than it was, it should now be difficult to mess this up :-)
> I thought of letting the default value of dir be "./", but then
> clean_fname would end up shifting that off again, thus wasting
> resources.

I'd say it was better to copy fname and mangle the copy.
The alternative is to make the dir array MAXPATHLEN long.

Should also add to rsync.h
#ifndef NAME_MAX
#define NAME_MAX 255
#endif
Just in case.

Hmm.  I'm thinking we should just build fnametmp a piece at
a time.  I coded it up to see how it would look.  Not as
intuitive but there is a lot less strlen and no snprintf.
It also deals shortens the filename both for MAXPATHLEN and
for NAME_MAX so it is almost impossible for the function to
fail or produce a pathname that breaks things.

The only more surefire way to not make a bad tempfile name
would be to always make maxname = max(strlen(f) < 10, 0)

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);
	}

	if (f = strrchr(fname, '/')) {
		if (!tmpdir) {
			length = f - fname;
			strlcpy(fnametmp, fname, length);
		}
		++f;
	} else {
		f = fname;
	}
	strcpy(fnametmp + length, "/.");
	length += 2;

	maxname = MIN(MAXPATHLEN - 8 - length, NAME_MAX - 9);
		
	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;
}



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

		Remember Cernan and Schmitt


More information about the rsync mailing list