From: Adam Roben <aroben@apple.com>
Date: Sun, 29 Apr 2007 20:40:19 GMT
Subject: Re: [PATCH 1/5] Make xstrndup common
Message-ID: <84F8DDF3-BCF5-4C2A-9C90-38AE139D7178@apple.com>
In-Reply-To: <4635003F.7080408@freedesktop.org>

On Apr 29, 2007, at 1:29 PM, Josh Triplett wrote:

> 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.

    An easy way around that is to do the string copy yourself,  
walking the string until you either find '\0' or reach len copied  
characters.

-Adam