[PATCH 1/5] Make xstrndup common

Subsystems: the rest

DORMANTno replies

8 messages, 5 authors, 2016-06-15 · open the first message on its own page

[PATCH 1/5] Make xstrndup common

From: Daniel Barkalow <hidden>
Date: 2016-06-15 22:43:07

It was implemented in commit.c; move it with the other x memory functions.

Signed-off-by: Daniel Barkalow <redacted>
---
 commit.c          |    8 --------
 git-compat-util.h |    8 ++++++++
 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/commit.c b/commit.c
index f1ba972..aa7059c 100644
--- a/commit.c
+++ b/commit.c
@@ -718,14 +718,6 @@ static char *logmsg_reencode(const struct commit *commit,
 	return out;
 }
 
-static char *xstrndup(const char *text, int len)
-{
-	char *result = xmalloc(len + 1);
-	memcpy(result, text, len);
-	result[len] = '\0';
-	return result;
-}
-
 static void fill_person(struct interp *table, const char *msg, int len)
 {
 	int start, end, tz = 0;
diff --git a/git-compat-util.h b/git-compat-util.h
index 2c84016..615c353 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -197,6 +197,14 @@ static inline void *xmalloc(size_t size)
 	return ret;
 }
 
+static inline char *xstrndup(const char *str, int len)
+{
+	char *ret = xmalloc(len + 1);
+	memcpy(ret, str, len);
+	ret[len] = '\0';
+	return ret;
+}
+
 static inline void *xrealloc(void *ptr, size_t size)
 {
 	void *ret = realloc(ptr, size);
-- 
1.5.1.2.255.g6ead4-dirty

Re: [PATCH 1/5] Make xstrndup common

From: Josh Triplett <hidden>
Date: 2016-06-15 22:43:07

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.

- Josh Triplett

Re: [PATCH 1/5] Make xstrndup common

From: Daniel Barkalow <hidden>
Date: 2016-06-15 22:43:07

On Sat, 28 Apr 2007, Josh Triplett wrote:
Daniel Barkalow wrote:
quoted
It was implemented in commit.c; move it with the other x memory functions.
[...]
quoted
+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).

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 1/5] Make xstrndup common

From: Josh Triplett <hidden>
Date: 2016-06-15 22:43:07

Daniel Barkalow wrote:
On Sat, 28 Apr 2007, Josh Triplett wrote:
quoted
Daniel Barkalow wrote:
quoted
It was implemented in commit.c; move it with the other x memory functions.
[...]
quoted
+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

Re: [PATCH 1/5] Make xstrndup common

From: Adam Roben <hidden>
Date: 2016-06-15 22:43:07

On Apr 29, 2007, at 1:29 PM, Josh Triplett wrote:
Daniel Barkalow wrote:
quoted
On Sat, 28 Apr 2007, Josh Triplett wrote:
quoted
Daniel Barkalow wrote:
quoted
It was implemented in commit.c; move it with the other x memory  
functions.
[...]
quoted
+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

Re: [PATCH 1/5] Make xstrndup common

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:07

Hi,

On Sun, 29 Apr 2007, Josh Triplett wrote:
Daniel Barkalow wrote:
quoted
On Sat, 28 Apr 2007, Josh Triplett wrote:
quoted
Daniel Barkalow wrote:
quoted
It was implemented in commit.c; move it with the other x memory functions.
[...]
quoted
+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.

Re: [PATCH 1/5] Make xstrndup common

From: Jeff King <hidden>
Date: 2016-06-15 22:43:07

On Mon, Apr 30, 2007 at 03:12:50PM +0200, Johannes Schindelin wrote:
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?
I can't speak for the original authors, but I imagine part of the
impetus was that strndup is a GNU-ism.

-Peff

Re: [PATCH 1/5] Make xstrndup common

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:07

Hi,

On Mon, 30 Apr 2007, Jeff King wrote:
On Mon, Apr 30, 2007 at 03:12:50PM +0200, Johannes Schindelin wrote:
quoted
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?
I can't speak for the original authors, but I imagine part of the
impetus was that strndup is a GNU-ism.
D'oh! I never thought this was a GNU-ism.

Ciao,
Dscho
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help