Re: [PATCH 1/5] Make xstrndup common

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 30, 2007, 13:12 UTC
Message-ID: <Pine.LNX.4.64.0704301508550.29859@racer.site>
In-Reply-To: <4635003F.7080408@freedesktop.org>

Hi,

On Sun, 29 Apr 2007, Josh Triplett wrote:

Show 27 quoted lines

> Daniel Barkalow wrote:
> > On Sat, 28 Apr 2007, Josh Triplett wrote:
> >> Daniel Barkalow wrote:
> >>> It was implemented in commit.c; move it with the other x memory functions.
> >> [...]
> >>> +static inline char *xstrndup(const char *str, int len)
> >>> +{
> >>> +	char *ret = xmalloc(len + 1);
> >>> +	memcpy(ret, str, len);
> >>> +	ret[len] = '\0';
> >>> +	return ret;
> >>> +}
> >>> +
> >> I don't know if it matters, but this definition of xstrndup, like the 
> >> version in commit.c, doesn't match the definition of strndup.  
> >> strndup duplicates a string, copying up to n characters or the length 
> >> of the string.  This xstrndup always copies n characters, reading 
> >> past the end of the string if it doesn't have at least n characters.
> > 
> > Good catch. Replacing the memcpy with strncpy solves this, right? 
> > (Potentially allocating a bit of extra memory if someone is actually 
> > using it on too short a string for some reason, of course).
> 
> That would work, but it seems bad to allocate excess memory.  How about 
> just using strlen and setting len to that if shorter, before doing the 
> xmalloc and memcpy?  Yes, that makes two passes over the string, but I 
> don't see any way around that.

Unless I am missing something, I think this should work:

static inline char *xstrndup(const char *str, int len)
{
	char *result = strndup(str, len);
	if (result == NULL)
		die ("xstrndup(): out of memory");
	return result;
}

Hmm?

Ciao, Dscho

P.S.: If you feel real paranoid about it, you might insert

	if (result == NULL) {
		release_pack_memory(len, -1);
		result = strndup(str, len);
	}

before the if (...), but I think that's overkill.