Re: New versions of openbsd_{basename,dirname}.c

[Samuel Thibault <sthibault@debian.org>]

Svante Signell, le Tue 17 Jan 2012 09:04:45 +0100, a écrit :
> 	static char *bname = NULL;
> 	register const char *endp, *startp;
> 
> 	bname = strdup(path);

You need to free bname before assigning something else, otherwise
nothing will ever free it. There is also no need for strdup, use
malloc(strlen(path)+1), since in all cases something will be strcpy-ed to
it. Also, put it below this:

> 	/* Empty or NULL string gets treated as "." */
> 	if (path == NULL || *path == '\0') {
> 		(void)strcpy(bname, ".");
> 		return(bname);
> 	}

Since otherwise when path is NULL strdup or strlen will just segfault;
Of course you'll need to replace strcpy(bname,".") with free(bname);
bname = strdup(".").

> 	/* Strip trailing slashes */
> 	endp = path + strlen(path) - 1;
> 	while (endp > path && *endp == '/')
> 		endp--;
> 
> 	/* All slashes becomes "/" */
> 	if (endp == path && *endp == '/') {
> 		(void)strcpy(bname, "/");
> 		return(bname);
> 	}
> 
> 	/* Find the start of the base */
> 	startp = endp;
> 	while (startp > path && *(startp - 1) != '/')
> 		startp--;
> 
> 	if (endp - startp + 1 > sizeof(bname)) {

Drop this test, it has become completely bogus now that bname is a
pointer, sizeof(bname) will just be typically 4.

> 		errno = ENAMETOOLONG;
> 		return(NULL);
> 	}
> 	(void)strncpy(bname, startp, endp - startp + 1);
> 	bname[endp - startp + 1] = '\0';
> 	return(bname);
> }



> 	const char *path;
> {
> 	static char *bname = NULL;
> 	register const char *endp;
> 
> 	/* Empty or NULL string gets treated as "." */
> 	if (path == NULL || *path == '\0') {

Ditto: you need to free the previous value of bname first:

> 		bname = malloc(2);
> 		(void)strcpy(bname, ".");

and you could just use bname = strdup(".") instead.

> 		return(bname);
> 	}
> 
> 	bname = strdup(path);

Ditto, replace this with a malloc.

> 	/* Strip trailing slashes */
> 	endp = path + strlen(path) - 1;
> 	while (endp > path && *endp == '/')
> 		endp--;
> 
> 	/* Find the start of the dir */
> 	while (endp > path && *endp != '/')
> 		endp--;
> 
> 	/* Either the dir is "/" or there are no slashes */
> 	if (endp == path) {
> 		(void)strcpy(bname, *endp == '/' ? "/" : ".");
> 		return(bname);
> 	} else {
> 		do {
> 			endp--;
> 		} while (endp > path && *endp == '/');
> 	}
> 
> 	if (endp - path + 1 > strlen(bname)) {

Ditto. This test is now meaningless.

> 		errno = ENAMETOOLONG;
> 		return(NULL);
> 	}
> 	(void)strncpy(bname, path, endp - path + 1);
> 	bname[endp - path + 1] = '\0';
> 	return(bname);
> }