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

[Samuel Thibault <sthibault@debian.org>]

Svante Signell, le Tue 17 Jan 2012 12:37:53 +0100, a écrit :
> On Tue, 2012-01-17 at 11:05 +0100, Samuel Thibault wrote:
> > In addition to that, it might be better for performance to rather
> > remember how much memory was previously allocated, and only reallocate
> > if more memory is needed, and twice as much, so as to reduce the amount
> > of free/malloc calls.
> 
> I don't think this kind of optimization is needed for the libtar
> application.

Why? "tar" is a performance-sensitive feature. I guess basename/dirname
is used quite a lot there.

> It definitely complicates the code.

Sure. Optimization often does. As long as it's bearable it's ok and
useful: here it would mean do as described below in the code (which,
unmodified, seems correct to me).

> ==14515==    still reachable: 4 bytes in 2 blocks

still reachable, yes, that's not an error. glibc does that in quite a
few places.

> ==14515==         suppressed: 0 bytes in 0 blocks
> ==14515== Rerun with --leak-check=full to see details of leaked memory
> ==14515== 
> ==14515== For counts of detected and suppressed errors, rerun with: -v
> ==14515== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4)



> openbsd_basename_new(path)
> 	const char *path;
> {
> 	static char *bname = NULL;

static unsigned allocated = 0;

> 	register const char *endp, *startp;
> 	int len = 0;

if (!allocated) {
  allocated = 64;
  bname = malloc(allocated);
  if (!bname) {
    allocated = 0;
    return NULL;
  }
}

> 	/* Empty or NULL string gets treated as "." */
> 	if (path == NULL || *path == '\0') {

strcpy(bname, ".");

> 		return(bname);
> 	}
> 
> 	/* Strip trailing slashes */
> 	endp = path + len - 1;
> 	while (endp > path && *endp == '/')
> 		endp--;
> 
> 	/* All slashes becomes "/" */
> 	if (endp == path && *endp == '/') {

strcpy(bname, "/");

> 		return(bname);
> 	}
> 
> 	/* Find the start of the base */
> 	startp = endp;
> 	while (startp > path && *(startp - 1) != '/')
> 		startp--;
> 
> 	len = endp - startp + 1;

if (len + 1 > allocated) {
	unsigned new_allocated = 2 * allocated;
	void *new_bname = malloc(new_allocated);
	if (!new_bname)
		return NULL;
	allocated = new_allocated;
	free(bname);
	bname = new_bname;
}
> 	(void)strncpy(bname, startp, len);
> 	bname[len] = '\0';
> 	return(bname);
> }