{"thread":{"id":"43997","subject":"[PATCH] compat: move strdup(3) replacement to its own file","startedAt":"2016-09-03T16:01:42Z","lastAt":"2016-09-08T06:59:41Z","messageCount":5,"participants":["René Scharfe","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"300955","messageId":"89725a44-8afa-1eb1-732a-23b1e264616c@web.de","threadId":"43997","inReplyTo":null,"subject":"[PATCH] compat: move strdup(3) replacement to its own file","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-09-03T15:59:15Z","receivedAt":"2016-09-03T16:01:42Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Move our implementation of strdup(3) out of compat/nedmalloc/ and allow\nit to be used independently from USE_NED_ALLOCATOR.  This reduces the\ndifference of our copy of nedmalloc from the original, making it easier\nto update, and allows for easier testing and reusing of our version of\nstrdup().\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n Makefile                     | 17 ++++++++++++++---\n compat/nedmalloc/nedmalloc.c | 16 ----------------\n compat/strdup.c              | 11 +++++++++++\n git-compat-util.h            |  8 ++++++++\n 4 files changed, 33 insertions(+), 19 deletions(-)\n create mode 100644 compat/strdup.c\n\ndiff --git a/Makefile b/Makefile\nindex d96ecb7..7f18492 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -296,6 +296,11 @@ all::\n # Define USE_NED_ALLOCATOR if you want to replace the platforms default\n # memory allocators with the nedmalloc allocator written by Niall Douglas.\n #\n+# Define OVERRIDE_STRDUP to override the libc version of strdup(3).\n+# This is necessary when using a custom allocator in order to avoid\n+# crashes due to allocation and free working on different 'heaps'.\n+# It's defined automatically if USE_NED_ALLOCATOR is set.\n+#\n # Define NO_REGEX if you have no or inferior regex support in your C library.\n #\n # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the\n@@ -1456,8 +1461,14 @@ ifdef NATIVE_CRLF\n endif\n \n ifdef USE_NED_ALLOCATOR\n-       COMPAT_CFLAGS += -Icompat/nedmalloc\n-       COMPAT_OBJS += compat/nedmalloc/nedmalloc.o\n+\tCOMPAT_CFLAGS += -Icompat/nedmalloc\n+\tCOMPAT_OBJS += compat/nedmalloc/nedmalloc.o\n+\tOVERRIDE_STRDUP = YesPlease\n+endif\n+\n+ifdef OVERRIDE_STRDUP\n+\tCOMPAT_CFLAGS += -DOVERRIDE_STRDUP\n+\tCOMPAT_OBJS += compat/strdup.o\n endif\n \n ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT\n@@ -2029,7 +2040,7 @@ endif\n \n ifdef USE_NED_ALLOCATOR\n compat/nedmalloc/nedmalloc.sp compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \\\n-\t-DNDEBUG -DOVERRIDE_STRDUP -DREPLACE_SYSTEM_ALLOCATOR\n+\t-DNDEBUG -DREPLACE_SYSTEM_ALLOCATOR\n compat/nedmalloc/nedmalloc.sp: SPARSE_FLAGS += -Wno-non-pointer-null\n endif\n \ndiff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c\nindex 2d4ef59..1cc31c3 100644\n--- a/compat/nedmalloc/nedmalloc.c\n+++ b/compat/nedmalloc/nedmalloc.c\n@@ -948,22 +948,6 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **\n \treturn ret;\n }\n \n-#ifdef OVERRIDE_STRDUP\n-/*\n- * This implementation is purely there to override the libc version, to\n- * avoid a crash due to allocation and free on different 'heaps'.\n- */\n-char *strdup(const char *s1)\n-{\n-\tsize_t len = strlen(s1) + 1;\n-\tchar *s2 = malloc(len);\n-\n-\tif (s2)\n-\t\tmemcpy(s2, s1, len);\n-\treturn s2;\n-}\n-#endif\n-\n #if defined(__cplusplus)\n }\n #endif\ndiff --git a/compat/strdup.c b/compat/strdup.c\nnew file mode 100644\nindex 0000000..f3fb978\n--- /dev/null\n+++ b/compat/strdup.c\n@@ -0,0 +1,11 @@\n+#include \"../git-compat-util.h\"\n+\n+char *gitstrdup(const char *s1)\n+{\n+\tsize_t len = strlen(s1) + 1;\n+\tchar *s2 = malloc(len);\n+\n+\tif (s2)\n+\t\tmemcpy(s2, s1, len);\n+\treturn s2;\n+}\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex db89ba7..93f6f23 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -663,6 +663,14 @@ void *gitmemmem(const void *haystack, size_t haystacklen,\n                 const void *needle, size_t needlelen);\n #endif\n \n+#ifdef OVERRIDE_STRDUP\n+#ifdef strdup\n+#undef strdup\n+#endif\n+#define strdup gitstrdup\n+char *gitstrdup(const char *s);\n+#endif\n+\n #ifdef NO_GETPAGESIZE\n #define getpagesize() sysconf(_SC_PAGESIZE)\n #endif\n-- \n2.10.0\n\n"},{"id":"300972","messageId":"alpine.DEB.2.20.1609040941210.129229@virtualbox","threadId":"43997","inReplyTo":"89725a44-8afa-1eb1-732a-23b1e264616c@web.de","subject":"Re: [PATCH] compat: move strdup(3) replacement to its own file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-04T07:46:31Z","receivedAt":"2016-09-04T07:55:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi René,\n\nI imagine you Cc:ed me because the nedmalloc stuff came in via the Windows\nport, contributed by Marius (who is no longer active on the Git project\nbecause it works well enough for him)?\n\nOn Sat, 3 Sep 2016, René Scharfe wrote:\n\n> Move our implementation of strdup(3) out of compat/nedmalloc/ and allow\n> it to be used independently from USE_NED_ALLOCATOR.  This reduces the\n> difference of our copy of nedmalloc from the original, making it easier\n> to update, and allows for easier testing and reusing of our version of\n> strdup().\n\nI would like to suggest an additional paragraph to explain why we do not\nneed to #include \"git-compat-util.h\" in nedmalloc from now on:\n\n\tPlease note that nedmalloc never actually uses strdup() itself,\n\ttherefore we need not enforce gitstrdup() usage in nedmalloc.c.\n\nThe patch looks quite straight-forward otherwise. (Junio, if you want an\nACK from me, you hereby got it).\n\nThanks!\nDscho"},{"id":"301194","messageId":"6926b39c-4448-c463-33f7-d9eae841c635@web.de","threadId":"43997","inReplyTo":"alpine.DEB.2.20.1609040941210.129229@virtualbox","subject":"Re: [PATCH] compat: move strdup(3) replacement to its own file","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-09-06T15:40:49Z","receivedAt":"2016-09-06T15:41:13Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 04.09.2016 um 09:46 schrieb Johannes Schindelin:\n> Hi René,\n>\n> I imagine you Cc:ed me because the nedmalloc stuff came in via the Windows\n> port, contributed by Marius (who is no longer active on the Git project\n> because it works well enough for him)?\n\nKind of; it's also a follow-up to the recent discussion you started \nabout compiler warnings in that code.\n\n> On Sat, 3 Sep 2016, René Scharfe wrote:\n>\n>> Move our implementation of strdup(3) out of compat/nedmalloc/ and allow\n>> it to be used independently from USE_NED_ALLOCATOR.  This reduces the\n>> difference of our copy of nedmalloc from the original, making it easier\n>> to update, and allows for easier testing and reusing of our version of\n>> strdup().\n>\n> I would like to suggest an additional paragraph to explain why we do not\n> need to #include \"git-compat-util.h\" in nedmalloc from now on:\n>\n> \tPlease note that nedmalloc never actually uses strdup() itself,\n> \ttherefore we need not enforce gitstrdup() usage in nedmalloc.c.\n\nWell, OK.  I think the missing point is that the original nedmalloc \ndoesn't come with strdup() and doesn't need it.  Only _users_ of \nnedmalloc need it.  Marius added it in nedmalloc.c, but strdup.c is a \nbetter place for it.\n\nRené\n"},{"id":"301298","messageId":"xmqqfupbob9c.fsf@gitster.mtv.corp.google.com","threadId":"43997","inReplyTo":"6926b39c-4448-c463-33f7-d9eae841c635@web.de","subject":"Re: [PATCH] compat: move strdup(3) replacement to its own file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-07T17:44:31Z","receivedAt":"2016-09-07T17:44:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Well, OK.  I think the missing point is that the original nedmalloc\n> doesn't come with strdup() and doesn't need it.  Only _users_ of\n> nedmalloc need it.  Marius added it in nedmalloc.c, but strdup.c is a\n> better place for it.\n\nThanks.  I'll add these lines like so:\n\n    Move our implementation of strdup(3) out of compat/nedmalloc/ and\n    allow it to be used independently from USE_NED_ALLOCATOR.  The\n    original nedmalloc doesn't come with strdup() and doesn't need it.\n    Only _users_ of nedmalloc need it, which was added when we imported\n    it to our compat/ hierarchy.\n\n    This reduces the difference of our copy of nedmalloc from the\n    original, making it easier to update, and allows for easier testing\n    and reusing of our version of strdup().\n"},{"id":"301370","messageId":"alpine.DEB.2.20.1609080859160.129229@virtualbox","threadId":"43997","inReplyTo":"xmqqfupbob9c.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] compat: move strdup(3) replacement to its own file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-08T06:59:27Z","receivedAt":"2016-09-08T06:59:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 Sep 2016, Junio C Hamano wrote:\n\n> René Scharfe <l.s.r@web.de> writes:\n> \n> > Well, OK.  I think the missing point is that the original nedmalloc\n> > doesn't come with strdup() and doesn't need it.  Only _users_ of\n> > nedmalloc need it.  Marius added it in nedmalloc.c, but strdup.c is a\n> > better place for it.\n> \n> Thanks.  I'll add these lines like so:\n> \n>     Move our implementation of strdup(3) out of compat/nedmalloc/ and\n>     allow it to be used independently from USE_NED_ALLOCATOR.  The\n>     original nedmalloc doesn't come with strdup() and doesn't need it.\n>     Only _users_ of nedmalloc need it, which was added when we imported\n>     it to our compat/ hierarchy.\n> \n>     This reduces the difference of our copy of nedmalloc from the\n>     original, making it easier to update, and allows for easier testing\n>     and reusing of our version of strdup().\n\nExcellent!\n\nThanks,\nDscho"}]}