git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Josh Triplett <josh@freedesktop.org>
To: Daniel Barkalow <barkalow@iabervon.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/5] Make xstrndup common
Date: Sun, 29 Apr 2007 13:29:51 -0700	[thread overview]
Message-ID: <4635003F.7080408@freedesktop.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0704291416370.28708@iabervon.org>

[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]

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.

I just checked the glibc source for strndup, and it does exactly the same
thing, except that it uses the glibc-specific function strnlen rather than
using strlen and figuring out the smaller of the two lengths.  That probably
increases efficiency if we have a string longer than, but we can't portably
use strnlen, so we'd have to check for it; doesn't seem worth the trouble.

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

  reply	other threads:[~2007-04-29 20:30 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-28 18:53 [PATCH 1/5] Make xstrndup common Josh Triplett
2007-04-28 22:47 ` Junio C Hamano
2007-04-29 18:19 ` Daniel Barkalow
2007-04-29 20:29   ` Josh Triplett [this message]
2007-04-29 20:39     ` Junio C Hamano
2007-04-29 20:40     ` Adam Roben
2007-04-30 13:12     ` Johannes Schindelin
2007-04-30 20:50       ` Jeff King
2007-04-30 22:21         ` Johannes Schindelin
  -- strict thread matches above, loose matches on Subject: below --
2007-04-28 17:05 Daniel Barkalow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4635003F.7080408@freedesktop.org \
    --to=josh@freedesktop.org \
    --cc=barkalow@iabervon.org \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).