test-mergesort conflict with stdlib.h

test-mergesort conflict with stdlib.h

[Brian Gernhardt]

On OS X 10.7.3, I get the following error attempting to build next:

    CC test-mergesort.o
In file included from test-mergesort.c:2:
./mergesort.h:4:7: error: conflicting types for 'mergesort'
void *mergesort(void *list,
      ^
/usr/include/stdlib.h:306:6: note: previous declaration is here
int      mergesort(void *, size_t, size_t,
         ^

This appears to be due to the fact that 0db71e0: "add mergesort() for linked lists" added a mergesort implementation with a different signature than a mergesort implemented by OS X's standard library.  Perhaps git could use a less generic name like 'llist_mergesort'?

~~ Brian Gernhardt

Re: test-mergesort conflict with stdlib.h

[Junio C Hamano]

Brian Gernhardt <brian@gernhardtsoftware.com> writes:

> On OS X 10.7.3, I get the following error attempting to build next:
>
>     CC test-mergesort.o
> In file included from test-mergesort.c:2:
> ./mergesort.h:4:7: error: conflicting types for 'mergesort'
> void *mergesort(void *list,
>       ^
> /usr/include/stdlib.h:306:6: note: previous declaration is here
> int      mergesort(void *, size_t, size_t,
>          ^
>
> This appears to be due to the fact that 0db71e0: "add mergesort() for
> linked lists" added a mergesort implementation with a different
> signature than a mergesort implemented by OS X's standard library.

Unfortunate.  OS X shouldn't contaminate the user namespace by adding such
names in stdlib.h (which incidentally has qsort()), but the damage is done
already, so I am not opposed to rename it to llist_mergesort() or something.

Re: test-mergesort conflict with stdlib.h

[Brian Gernhardt]

On Apr 17, 2012, at 1:46 PM, Junio C Hamano wrote:

> Unfortunate.  OS X shouldn't contaminate the user namespace by adding such
> names in stdlib.h (which incidentally has qsort()), but the damage is done
> already, so I am not opposed to rename it to llist_mergesort() or something.

This seems to be inherited from BSD's userspace which also has a mergesort (radixsort as well).  I'll make a patch to rename it and if anyone doesn't like llist_mergesort, they can repaint the bikeshed themselves.

~~ Brian Gernhardt

Re: test-mergesort conflict with stdlib.h

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Unfortunate.  OS X shouldn't contaminate the user namespace by adding such
> names in stdlib.h (which incidentally has qsort()), but the damage is done
> already, so I am not opposed to rename it to llist_mergesort() or something.

Something like this.  While we are touching the code, let's add some
documentation to the header.

 commit.c         |    4 ++--
 mergesort.c      |    8 ++++----
 mergesort.h      |   16 ++++++++++++----
 test-mergesort.c |    2 +-
 4 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/commit.c b/commit.c
index 0759b2c..84304c0 100644
--- a/commit.c
+++ b/commit.c
@@ -429,8 +429,8 @@ static void commit_list_set_next(void *a, void *next)
 
 void commit_list_sort_by_date(struct commit_list **list)
 {
-	*list = mergesort(*list, commit_list_get_next, commit_list_set_next,
-			  commit_list_compare_by_date);
+	*list = llist_mergesort(*list, commit_list_get_next, commit_list_set_next,
+				commit_list_compare_by_date);
 }
 
 struct commit *pop_most_recent_commit(struct commit_list **list,
diff --git a/mergesort.c b/mergesort.c
index d084c60..e5fdf2e 100644
--- a/mergesort.c
+++ b/mergesort.c
@@ -23,10 +23,10 @@ static void *pop_item(struct mergesort_sublist *l,
 	return p;
 }
 
-void *mergesort(void *list,
-		void *(*get_next_fn)(const void *),
-		void (*set_next_fn)(void *, void *),
-		int (*compare_fn)(const void *, const void *))
+void *llist_mergesort(void *list,
+		      void *(*get_next_fn)(const void *),
+		      void (*set_next_fn)(void *, void *),
+		      int (*compare_fn)(const void *, const void *))
 {
 	unsigned long l;
 
diff --git a/mergesort.h b/mergesort.h
index d6e5f4a..644cff1 100644
--- a/mergesort.h
+++ b/mergesort.h
@@ -1,9 +1,17 @@
 #ifndef MERGESORT_H
 #define MERGESORT_H
 
-void *mergesort(void *list,
-		void *(*get_next_fn)(const void *),
-		void (*set_next_fn)(void *, void *),
-		int (*compare_fn)(const void *, const void *));
+/*
+ * Sort linked list in place.
+ * - get_next_fn() returns the next element given an element of a linked list.
+ * - set_next_fn() takes two elements A and B, and makes B the "next" element
+ *   of A on the list.
+ * - compare_fn() takes two elements A and B, and returns negative, 0, positive
+ *   as the same sign as "subtracting" B from A.
+ */
+void *llist_mergesort(void *list,
+		      void *(*get_next_fn)(const void *),
+		      void (*set_next_fn)(void *, void *),
+		      int (*compare_fn)(const void *, const void *));
 
 #endif
diff --git a/test-mergesort.c b/test-mergesort.c
index 1dd82fd..3f388b4 100644
--- a/test-mergesort.c
+++ b/test-mergesort.c
@@ -42,7 +42,7 @@ int main(int argc, const char **argv)
 		p = line;
 	}
 
-	lines = mergesort(lines, get_next, set_next, compare_strings);
+	lines = llist_mergesort(lines, get_next, set_next, compare_strings);
 
 	while (lines) {
 		printf("%s", lines->text);

Re: test-mergesort conflict with stdlib.h

[Brian Gernhardt]

On Apr 17, 2012, at 2:06 PM, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Unfortunate.  OS X shouldn't contaminate the user namespace by adding such
>> names in stdlib.h (which incidentally has qsort()), but the damage is done
>> already, so I am not opposed to rename it to llist_mergesort() or something.
> 
> Something like this.  While we are touching the code, let's add some
> documentation to the header.
> 
> commit.c         |    4 ++--
> mergesort.c      |    8 ++++----
> mergesort.h      |   16 ++++++++++++----
> test-mergesort.c |    2 +-
> 4 files changed, 19 insertions(+), 11 deletions(-)

This is basically identical to what I was going to send and it does, in fact, fix my build problem.

Thanks,
~~ Brian Gernhardt