[PATCH] compat: move strdup(3) replacement to its own file

[PATCH] compat: move strdup(3) replacement to its own file

[René Scharfe]

Move our implementation of strdup(3) out of compat/nedmalloc/ and allow
it to be used independently from USE_NED_ALLOCATOR.  This reduces the
difference of our copy of nedmalloc from the original, making it easier
to update, and allows for easier testing and reusing of our version of
strdup().

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 Makefile                     | 17 ++++++++++++++---
 compat/nedmalloc/nedmalloc.c | 16 ----------------
 compat/strdup.c              | 11 +++++++++++
 git-compat-util.h            |  8 ++++++++
 4 files changed, 33 insertions(+), 19 deletions(-)
 create mode 100644 compat/strdup.c

diff --git a/Makefile b/Makefile
index d96ecb7..7f18492 100644
--- a/Makefile
+++ b/Makefile
@@ -296,6 +296,11 @@ all::
 # Define USE_NED_ALLOCATOR if you want to replace the platforms default
 # memory allocators with the nedmalloc allocator written by Niall Douglas.
 #
+# Define OVERRIDE_STRDUP to override the libc version of strdup(3).
+# This is necessary when using a custom allocator in order to avoid
+# crashes due to allocation and free working on different 'heaps'.
+# It's defined automatically if USE_NED_ALLOCATOR is set.
+#
 # Define NO_REGEX if you have no or inferior regex support in your C library.
 #
 # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
@@ -1456,8 +1461,14 @@ ifdef NATIVE_CRLF
 endif
 
 ifdef USE_NED_ALLOCATOR
-       COMPAT_CFLAGS += -Icompat/nedmalloc
-       COMPAT_OBJS += compat/nedmalloc/nedmalloc.o
+	COMPAT_CFLAGS += -Icompat/nedmalloc
+	COMPAT_OBJS += compat/nedmalloc/nedmalloc.o
+	OVERRIDE_STRDUP = YesPlease
+endif
+
+ifdef OVERRIDE_STRDUP
+	COMPAT_CFLAGS += -DOVERRIDE_STRDUP
+	COMPAT_OBJS += compat/strdup.o
 endif
 
 ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
@@ -2029,7 +2040,7 @@ endif
 
 ifdef USE_NED_ALLOCATOR
 compat/nedmalloc/nedmalloc.sp compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \
-	-DNDEBUG -DOVERRIDE_STRDUP -DREPLACE_SYSTEM_ALLOCATOR
+	-DNDEBUG -DREPLACE_SYSTEM_ALLOCATOR
 compat/nedmalloc/nedmalloc.sp: SPARSE_FLAGS += -Wno-non-pointer-null
 endif
 
diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
index 2d4ef59..1cc31c3 100644
--- a/compat/nedmalloc/nedmalloc.c
+++ b/compat/nedmalloc/nedmalloc.c
@@ -948,22 +948,6 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **
 	return ret;
 }
 
-#ifdef OVERRIDE_STRDUP
-/*
- * This implementation is purely there to override the libc version, to
- * avoid a crash due to allocation and free on different 'heaps'.
- */
-char *strdup(const char *s1)
-{
-	size_t len = strlen(s1) + 1;
-	char *s2 = malloc(len);
-
-	if (s2)
-		memcpy(s2, s1, len);
-	return s2;
-}
-#endif
-
 #if defined(__cplusplus)
 }
 #endif
diff --git a/compat/strdup.c b/compat/strdup.c
new file mode 100644
index 0000000..f3fb978
--- /dev/null
+++ b/compat/strdup.c
@@ -0,0 +1,11 @@
+#include "../git-compat-util.h"
+
+char *gitstrdup(const char *s1)
+{
+	size_t len = strlen(s1) + 1;
+	char *s2 = malloc(len);
+
+	if (s2)
+		memcpy(s2, s1, len);
+	return s2;
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index db89ba7..93f6f23 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -663,6 +663,14 @@ void *gitmemmem(const void *haystack, size_t haystacklen,
                 const void *needle, size_t needlelen);
 #endif
 
+#ifdef OVERRIDE_STRDUP
+#ifdef strdup
+#undef strdup
+#endif
+#define strdup gitstrdup
+char *gitstrdup(const char *s);
+#endif
+
 #ifdef NO_GETPAGESIZE
 #define getpagesize() sysconf(_SC_PAGESIZE)
 #endif
-- 
2.10.0

Re: [PATCH] compat: move strdup(3) replacement to its own file

[Johannes Schindelin]

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

Hi René,

I imagine you Cc:ed me because the nedmalloc stuff came in via the Windows
port, contributed by Marius (who is no longer active on the Git project
because it works well enough for him)?

On Sat, 3 Sep 2016, René Scharfe wrote:

> Move our implementation of strdup(3) out of compat/nedmalloc/ and allow
> it to be used independently from USE_NED_ALLOCATOR.  This reduces the
> difference of our copy of nedmalloc from the original, making it easier
> to update, and allows for easier testing and reusing of our version of
> strdup().

I would like to suggest an additional paragraph to explain why we do not
need to #include "git-compat-util.h" in nedmalloc from now on:

	Please note that nedmalloc never actually uses strdup() itself,
	therefore we need not enforce gitstrdup() usage in nedmalloc.c.

The patch looks quite straight-forward otherwise. (Junio, if you want an
ACK from me, you hereby got it).

Thanks!
Dscho

Re: [PATCH] compat: move strdup(3) replacement to its own file

[René Scharfe]

Am 04.09.2016 um 09:46 schrieb Johannes Schindelin:
> Hi René,
>
> I imagine you Cc:ed me because the nedmalloc stuff came in via the Windows
> port, contributed by Marius (who is no longer active on the Git project
> because it works well enough for him)?

Kind of; it's also a follow-up to the recent discussion you started 
about compiler warnings in that code.

> On Sat, 3 Sep 2016, René Scharfe wrote:
>
>> Move our implementation of strdup(3) out of compat/nedmalloc/ and allow
>> it to be used independently from USE_NED_ALLOCATOR.  This reduces the
>> difference of our copy of nedmalloc from the original, making it easier
>> to update, and allows for easier testing and reusing of our version of
>> strdup().
>
> I would like to suggest an additional paragraph to explain why we do not
> need to #include "git-compat-util.h" in nedmalloc from now on:
>
> 	Please note that nedmalloc never actually uses strdup() itself,
> 	therefore we need not enforce gitstrdup() usage in nedmalloc.c.

Well, OK.  I think the missing point is that the original nedmalloc 
doesn't come with strdup() and doesn't need it.  Only _users_ of 
nedmalloc need it.  Marius added it in nedmalloc.c, but strdup.c is a 
better place for it.

René

Re: [PATCH] compat: move strdup(3) replacement to its own file

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> Well, OK.  I think the missing point is that the original nedmalloc
> doesn't come with strdup() and doesn't need it.  Only _users_ of
> nedmalloc need it.  Marius added it in nedmalloc.c, but strdup.c is a
> better place for it.

Thanks.  I'll add these lines like so:

    Move our implementation of strdup(3) out of compat/nedmalloc/ and
    allow it to be used independently from USE_NED_ALLOCATOR.  The
    original nedmalloc doesn't come with strdup() and doesn't need it.
    Only _users_ of nedmalloc need it, which was added when we imported
    it to our compat/ hierarchy.

    This reduces the difference of our copy of nedmalloc from the
    original, making it easier to update, and allows for easier testing
    and reusing of our version of strdup().

Re: [PATCH] compat: move strdup(3) replacement to its own file

[Johannes Schindelin]

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

Hi,

On Wed, 7 Sep 2016, Junio C Hamano wrote:

> René Scharfe <l.s.r@web.de> writes:
> 
> > Well, OK.  I think the missing point is that the original nedmalloc
> > doesn't come with strdup() and doesn't need it.  Only _users_ of
> > nedmalloc need it.  Marius added it in nedmalloc.c, but strdup.c is a
> > better place for it.
> 
> Thanks.  I'll add these lines like so:
> 
>     Move our implementation of strdup(3) out of compat/nedmalloc/ and
>     allow it to be used independently from USE_NED_ALLOCATOR.  The
>     original nedmalloc doesn't come with strdup() and doesn't need it.
>     Only _users_ of nedmalloc need it, which was added when we imported
>     it to our compat/ hierarchy.
> 
>     This reduces the difference of our copy of nedmalloc from the
>     original, making it easier to update, and allows for easier testing
>     and reusing of our version of strdup().

Excellent!

Thanks,
Dscho