unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#18360: Modularize recent qsort_r additions
@ 2014-08-29 20:58 Paul Eggert
  2014-08-30  7:29 ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Eggert @ 2014-08-29 20:58 UTC (permalink / raw)
  To: 18360

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

Tags: patch

The recent changes to have Emacs use qsort_r look good except for the 
forest of #ifdefs in Emacs proper.  Attached is a proposed patch to 
migrate that stuff into Gnulib.  As usual most of this patch is 
automatically generated; fns.c contains the crucial hand-generated 
stuff.  I have not tested this on Microsoft Windows and so am posting it 
here first to give Eli a heads-up.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: qsort_r.patch --]
[-- Type: text/x-patch; name="qsort_r.patch", Size: 26022 bytes --]

=== modified file 'ChangeLog'
--- ChangeLog	2014-08-29 07:29:47 +0000
+++ ChangeLog	2014-08-29 20:46:33 +0000
@@ -1,3 +1,13 @@
+2014-08-29  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Modularize qsort_r in Gnulib, to simplify Emacs proper.
+	* configure.ac (qsort_r): Remove check; gnulib does this now.
+	* lib/qsort.c, lib/qsort_r.c, m4/qsort_r.m4: New files.
+	Merge from gnulib, incorporating:
+	2014-08-29 qsort_r: new module, for GNU-style qsort_r
+	* lib/gnulib.mk, m4/gnulib-comp.m4: Regenerate.
+	* lib/stdlib.in.h, m4/stdlib_h.m4: Update from gnulib.
+
 2014-08-29  Dmitry Antipov  <dmantipov@yandex.ru>
 
 	* configure.ac (AC_CHECK_FUNCS): Check for qsort_r.

=== modified file 'admin/ChangeLog'
--- admin/ChangeLog	2014-08-29 18:10:15 +0000
+++ admin/ChangeLog	2014-08-29 20:46:33 +0000
@@ -1,3 +1,8 @@
+2014-08-29  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Modularize qsort_r in Gnulib, to simplify Emacs proper.
+	* merge-gnulib (GNULIB_MODULES): Add qsort_r.
+
 2014-08-29  Michael Albinus  <michael.albinus@gmx.de>
 
 	* authors.el (authors): Use LOCALE argument of `string-collate-lessp'.

=== modified file 'admin/merge-gnulib'
--- admin/merge-gnulib	2014-07-14 19:23:18 +0000
+++ admin/merge-gnulib	2014-08-29 20:46:33 +0000
@@ -34,7 +34,7 @@
   getloadavg getopt-gnu gettime gettimeofday
   intprops largefile lstat
   manywarnings memrchr mkostemp mktime
-  pipe2 pselect pthread_sigmask putenv qacl readlink readlinkat
+  pipe2 pselect pthread_sigmask putenv qacl qsort_r readlink readlinkat
   sig2str socklen stat-time stdalign stdio
   strftime strtoimax strtoumax symlink sys_stat
   sys_time time timer-time timespec-add timespec-sub

=== modified file 'configure.ac'
--- configure.ac	2014-08-29 07:29:47 +0000
+++ configure.ac	2014-08-29 20:46:33 +0000
@@ -3573,7 +3573,7 @@
 getrlimit setrlimit shutdown getaddrinfo \
 pthread_sigmask strsignal setitimer \
 sendto recvfrom getsockname getpeername getifaddrs freeifaddrs \
-gai_strerror sync qsort_r \
+gai_strerror sync \
 getpwent endpwent getgrent endgrent \
 cfmakeraw cfsetspeed copysign __executable_start log2)
 LIBS=$OLD_LIBS

=== modified file 'lib/gnulib.mk'
--- lib/gnulib.mk	2014-08-04 18:44:49 +0000
+++ lib/gnulib.mk	2014-08-29 20:46:33 +0000
@@ -21,7 +21,7 @@
 # the same distribution terms as the rest of that program.
 #
 # Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=close --avoid=dup --avoid=fchdir --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=open --avoid=openat-die --avoid=opendir --avoid=raise --avoid=save-cwd --avoid=select --avoid=sigprocmask --avoid=stdarg --avoid=stdbool --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt binary-io byteswap c-ctype c-strcase careadlinkat close-stream count-one-bits count-trailing-zeros crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo faccessat fcntl fcntl-h fdatasync fdopendir filemode fstatat fsync getloadavg getopt-gnu gettime gettimeofday intprops largefile lstat manywarnings memrchr mkostemp mktime pipe2 pselect pthread_sigmask putenv qacl readlink readlinkat sig2str socklen stat-time stdalign stdio strftime strtoimax strtoumax symlink sys_stat sys_time time timer-time timespec-add timespec-sub unsetenv update-copyright utimens warnings
+# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=close --avoid=dup --avoid=fchdir --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=open --avoid=openat-die --avoid=opendir --avoid=raise --avoid=save-cwd --avoid=select --avoid=sigprocmask --avoid=stdarg --avoid=stdbool --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt binary-io byteswap c-ctype c-strcase careadlinkat close-stream count-one-bits count-trailing-zeros crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo faccessat fcntl fcntl-h fdatasync fdopendir filemode fstatat fsync getloadavg getopt-gnu gettime gettimeofday intprops largefile lstat manywarnings memrchr mkostemp mktime pipe2 pselect pthread_sigmask putenv qacl qsort_r readlink readlinkat sig2str socklen stat-time stdalign stdio strftime strtoimax strtoumax symlink sys_stat sys_time time timer-time timespec-add timespec-sub unsetenv update-copyright utimens warnings
 
 
 MOSTLYCLEANFILES += core *.stackdump
@@ -688,6 +688,15 @@
 
 ## end   gnulib module qacl
 
+## begin gnulib module qsort_r
+
+
+EXTRA_DIST += qsort.c qsort_r.c
+
+EXTRA_libgnu_a_SOURCES += qsort.c qsort_r.c
+
+## end   gnulib module qsort_r
+
 ## begin gnulib module readlink
 
 
@@ -1141,6 +1150,7 @@
 	      -e 's/@''GNULIB_PTSNAME''@/$(GNULIB_PTSNAME)/g' \
 	      -e 's/@''GNULIB_PTSNAME_R''@/$(GNULIB_PTSNAME_R)/g' \
 	      -e 's/@''GNULIB_PUTENV''@/$(GNULIB_PUTENV)/g' \
+	      -e 's/@''GNULIB_QSORT_R''@/$(GNULIB_QSORT_R)/g' \
 	      -e 's/@''GNULIB_RANDOM''@/$(GNULIB_RANDOM)/g' \
 	      -e 's/@''GNULIB_RANDOM_R''@/$(GNULIB_RANDOM_R)/g' \
 	      -e 's/@''GNULIB_REALLOC_POSIX''@/$(GNULIB_REALLOC_POSIX)/g' \
@@ -1192,6 +1202,7 @@
 	      -e 's|@''REPLACE_PTSNAME''@|$(REPLACE_PTSNAME)|g' \
 	      -e 's|@''REPLACE_PTSNAME_R''@|$(REPLACE_PTSNAME_R)|g' \
 	      -e 's|@''REPLACE_PUTENV''@|$(REPLACE_PUTENV)|g' \
+	      -e 's|@''REPLACE_QSORT_R''@|$(REPLACE_QSORT_R)|g' \
 	      -e 's|@''REPLACE_RANDOM_R''@|$(REPLACE_RANDOM_R)|g' \
 	      -e 's|@''REPLACE_REALLOC''@|$(REPLACE_REALLOC)|g' \
 	      -e 's|@''REPLACE_REALPATH''@|$(REPLACE_REALPATH)|g' \

=== added file 'lib/qsort.c'
--- lib/qsort.c	1970-01-01 00:00:00 +0000
+++ lib/qsort.c	2014-08-29 20:46:33 +0000
@@ -0,0 +1,254 @@
+/* Copyright (C) 1991-2014 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Written by Douglas C. Schmidt (schmidt@ics.uci.edu).
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU General Public
+   License as published by the Free Software Foundation; either
+   version 3 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* If you consider tuning this algorithm, you should consult first:
+   Engineering a sort function; Jon Bentley and M. Douglas McIlroy;
+   Software - Practice and Experience; Vol. 23 (11), 1249-1265, 1993.  */
+
+#include <limits.h>
+#include <stdlib.h>
+#include <string.h>
+
+#ifndef _LIBC
+# define _quicksort qsort_r
+# define __compar_d_fn_t compar_d_fn_t
+typedef int (*compar_d_fn_t) (const void *, const void *, void *);
+#endif
+
+/* Byte-wise swap two items of size SIZE. */
+#define SWAP(a, b, size)						      \
+  do									      \
+    {									      \
+      size_t __size = (size);						      \
+      char *__a = (a), *__b = (b);					      \
+      do								      \
+	{								      \
+	  char __tmp = *__a;						      \
+	  *__a++ = *__b;						      \
+	  *__b++ = __tmp;						      \
+	} while (--__size > 0);						      \
+    } while (0)
+
+/* Discontinue quicksort algorithm when partition gets below this size.
+   This particular magic number was chosen to work best on a Sun 4/260. */
+#define MAX_THRESH 4
+
+/* Stack node declarations used to store unfulfilled partition obligations. */
+typedef struct
+  {
+    char *lo;
+    char *hi;
+  } stack_node;
+
+/* The next 4 #defines implement a very fast in-line stack abstraction. */
+/* The stack needs log (total_elements) entries (we could even subtract
+   log(MAX_THRESH)).  Since total_elements has type size_t, we get as
+   upper bound for log (total_elements):
+   bits per byte (CHAR_BIT) * sizeof(size_t).  */
+#define STACK_SIZE	(CHAR_BIT * sizeof(size_t))
+#define PUSH(low, high)	((void) ((top->lo = (low)), (top->hi = (high)), ++top))
+#define	POP(low, high)	((void) (--top, (low = top->lo), (high = top->hi)))
+#define	STACK_NOT_EMPTY	(stack < top)
+
+
+/* Order size using quicksort.  This implementation incorporates
+   four optimizations discussed in Sedgewick:
+
+   1. Non-recursive, using an explicit stack of pointer that store the
+      next array partition to sort.  To save time, this maximum amount
+      of space required to store an array of SIZE_MAX is allocated on the
+      stack.  Assuming a 32-bit (64 bit) integer for size_t, this needs
+      only 32 * sizeof(stack_node) == 256 bytes (for 64 bit: 1024 bytes).
+      Pretty cheap, actually.
+
+   2. Chose the pivot element using a median-of-three decision tree.
+      This reduces the probability of selecting a bad pivot value and
+      eliminates certain extraneous comparisons.
+
+   3. Only quicksorts TOTAL_ELEMS / MAX_THRESH partitions, leaving
+      insertion sort to order the MAX_THRESH items within each partition.
+      This is a big win, since insertion sort is faster for small, mostly
+      sorted array segments.
+
+   4. The larger of the two sub-partitions is always pushed onto the
+      stack first, with the algorithm then concentrating on the
+      smaller partition.  This *guarantees* no more than log (total_elems)
+      stack size is needed (actually O(1) in this case)!  */
+
+void
+_quicksort (void *const pbase, size_t total_elems, size_t size,
+	    __compar_d_fn_t cmp, void *arg)
+{
+  char *base_ptr = (char *) pbase;
+
+  const size_t max_thresh = MAX_THRESH * size;
+
+  if (total_elems == 0)
+    /* Avoid lossage with unsigned arithmetic below.  */
+    return;
+
+  if (total_elems > MAX_THRESH)
+    {
+      char *lo = base_ptr;
+      char *hi = &lo[size * (total_elems - 1)];
+      stack_node stack[STACK_SIZE];
+      stack_node *top = stack;
+
+      PUSH (NULL, NULL);
+
+      while (STACK_NOT_EMPTY)
+        {
+          char *left_ptr;
+          char *right_ptr;
+
+	  /* Select median value from among LO, MID, and HI. Rearrange
+	     LO and HI so the three values are sorted. This lowers the
+	     probability of picking a pathological pivot value and
+	     skips a comparison for both the LEFT_PTR and RIGHT_PTR in
+	     the while loops. */
+
+	  char *mid = lo + size * ((hi - lo) / size >> 1);
+
+	  if ((*cmp) ((void *) mid, (void *) lo, arg) < 0)
+	    SWAP (mid, lo, size);
+	  if ((*cmp) ((void *) hi, (void *) mid, arg) < 0)
+	    SWAP (mid, hi, size);
+	  else
+	    goto jump_over;
+	  if ((*cmp) ((void *) mid, (void *) lo, arg) < 0)
+	    SWAP (mid, lo, size);
+	jump_over:;
+
+	  left_ptr  = lo + size;
+	  right_ptr = hi - size;
+
+	  /* Here's the famous ``collapse the walls'' section of quicksort.
+	     Gotta like those tight inner loops!  They are the main reason
+	     that this algorithm runs much faster than others. */
+	  do
+	    {
+	      while ((*cmp) ((void *) left_ptr, (void *) mid, arg) < 0)
+		left_ptr += size;
+
+	      while ((*cmp) ((void *) mid, (void *) right_ptr, arg) < 0)
+		right_ptr -= size;
+
+	      if (left_ptr < right_ptr)
+		{
+		  SWAP (left_ptr, right_ptr, size);
+		  if (mid == left_ptr)
+		    mid = right_ptr;
+		  else if (mid == right_ptr)
+		    mid = left_ptr;
+		  left_ptr += size;
+		  right_ptr -= size;
+		}
+	      else if (left_ptr == right_ptr)
+		{
+		  left_ptr += size;
+		  right_ptr -= size;
+		  break;
+		}
+	    }
+	  while (left_ptr <= right_ptr);
+
+          /* Set up pointers for next iteration.  First determine whether
+             left and right partitions are below the threshold size.  If so,
+             ignore one or both.  Otherwise, push the larger partition's
+             bounds on the stack and continue sorting the smaller one. */
+
+          if ((size_t) (right_ptr - lo) <= max_thresh)
+            {
+              if ((size_t) (hi - left_ptr) <= max_thresh)
+		/* Ignore both small partitions. */
+                POP (lo, hi);
+              else
+		/* Ignore small left partition. */
+                lo = left_ptr;
+            }
+          else if ((size_t) (hi - left_ptr) <= max_thresh)
+	    /* Ignore small right partition. */
+            hi = right_ptr;
+          else if ((right_ptr - lo) > (hi - left_ptr))
+            {
+	      /* Push larger left partition indices. */
+              PUSH (lo, right_ptr);
+              lo = left_ptr;
+            }
+          else
+            {
+	      /* Push larger right partition indices. */
+              PUSH (left_ptr, hi);
+              hi = right_ptr;
+            }
+        }
+    }
+
+  /* Once the BASE_PTR array is partially sorted by quicksort the rest
+     is completely sorted using insertion sort, since this is efficient
+     for partitions below MAX_THRESH size. BASE_PTR points to the beginning
+     of the array to sort, and END_PTR points at the very last element in
+     the array (*not* one beyond it!). */
+
+#define min(x, y) ((x) < (y) ? (x) : (y))
+
+  {
+    char *const end_ptr = &base_ptr[size * (total_elems - 1)];
+    char *tmp_ptr = base_ptr;
+    char *thresh = min(end_ptr, base_ptr + max_thresh);
+    char *run_ptr;
+
+    /* Find smallest element in first threshold and place it at the
+       array's beginning.  This is the smallest array element,
+       and the operation speeds up insertion sort's inner loop. */
+
+    for (run_ptr = tmp_ptr + size; run_ptr <= thresh; run_ptr += size)
+      if ((*cmp) ((void *) run_ptr, (void *) tmp_ptr, arg) < 0)
+        tmp_ptr = run_ptr;
+
+    if (tmp_ptr != base_ptr)
+      SWAP (tmp_ptr, base_ptr, size);
+
+    /* Insertion sort, running from left-hand-side up to right-hand-side.  */
+
+    run_ptr = base_ptr + size;
+    while ((run_ptr += size) <= end_ptr)
+      {
+	tmp_ptr = run_ptr - size;
+	while ((*cmp) ((void *) run_ptr, (void *) tmp_ptr, arg) < 0)
+	  tmp_ptr -= size;
+
+	tmp_ptr += size;
+        if (tmp_ptr != run_ptr)
+          {
+            char *trav;
+
+	    trav = run_ptr + size;
+	    while (--trav >= run_ptr)
+              {
+                char c = *trav;
+                char *hi, *lo;
+
+                for (hi = lo = trav; (lo -= size) >= tmp_ptr; hi = lo)
+                  *hi = *lo;
+                *hi = c;
+              }
+          }
+      }
+  }
+}

=== added file 'lib/qsort_r.c'
--- lib/qsort_r.c	1970-01-01 00:00:00 +0000
+++ lib/qsort_r.c	2014-08-29 20:46:33 +0000
@@ -0,0 +1,51 @@
+/* Reentrant sort function.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License along
+   with this program; if not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Paul Eggert.  */
+
+#include <config.h>
+
+#include <stdlib.h>
+
+/* This file is compiled only when the system has a qsort_r that needs
+   to be replaced because it has the BSD signature rather than the GNU
+   signature.  */
+
+struct thunk
+{
+  int (*cmp) (void const *, void const *, void *);
+  void *arg;
+};
+
+static int
+thunk_cmp (void *thunk, void const *a, void const *b)
+{
+  struct thunk *th = thunk;
+  return th->cmp (a, b, th->arg);
+}
+
+void
+qsort_r (void *base, size_t nmemb, size_t size,
+         int (*cmp) (void const *, void const *, void *),
+         void *arg)
+{
+# undef qsort_r
+  struct thunk thunk;
+  thunk.cmp = cmp;
+  thunk.arg = arg;
+  qsort_r (base, nmemb, size, &thunk, thunk_cmp);
+}

=== modified file 'lib/stdlib.in.h'
--- lib/stdlib.in.h	2014-01-01 07:43:34 +0000
+++ lib/stdlib.in.h	2014-08-29 20:46:33 +0000
@@ -520,6 +520,29 @@
 _GL_CXXALIASWARN (putenv);
 #endif
 
+#if @GNULIB_QSORT_R@
+# if @REPLACE_QSORT_R@
+#  if !(defined __cplusplus && defined GNULIB_NAMESPACE)
+#   undef qsort_r
+#   define qsort_r rpl_qsort_r
+#  endif
+_GL_FUNCDECL_RPL (qsort_r, void, (void *base, size_t nmemb, size_t size,
+                                  int (*compare) (void const *, void const *,
+                                                  void *),
+                                  void *arg) _GL_ARG_NONNULL ((1, 4)));
+_GL_CXXALIAS_RPL (qsort_r, void, (void *base, size_t nmemb, size_t size,
+                                  int (*compare) (void const *, void const *,
+                                                  void *),
+                                  void *arg));
+# else
+_GL_CXXALIAS_SYS (qsort_r, void, (void *base, size_t nmemb, size_t size,
+                                  int (*compare) (void const *, void const *,
+                                                  void *),
+                                  void *arg));
+# endif
+_GL_CXXALIASWARN (qsort_r);
+#endif
+
 
 #if @GNULIB_RANDOM_R@
 # if !@HAVE_RANDOM_R@

=== modified file 'm4/gnulib-comp.m4'
--- m4/gnulib-comp.m4	2014-05-17 08:11:31 +0000
+++ m4/gnulib-comp.m4	2014-08-29 20:46:33 +0000
@@ -104,6 +104,7 @@
   # Code from module pthread_sigmask:
   # Code from module putenv:
   # Code from module qacl:
+  # Code from module qsort_r:
   # Code from module readlink:
   # Code from module readlinkat:
   # Code from module root-uid:
@@ -313,6 +314,13 @@
   fi
   gl_STDLIB_MODULE_INDICATOR([putenv])
   gl_FUNC_ACL
+  gl_FUNC_QSORT_R
+  if test $HAVE_QSORT_R = 0; then
+    AC_LIBOBJ([qsort])
+  elif test $REPLACE_QSORT_R = 1; then
+    AC_LIBOBJ([qsort_r])
+  fi
+  gl_STDLIB_MODULE_INDICATOR([qsort_r])
   gl_FUNC_READLINK
   if test $HAVE_READLINK = 0 || test $REPLACE_READLINK = 1; then
     AC_LIBOBJ([readlink])
@@ -865,6 +873,8 @@
   lib/putenv.c
   lib/qcopy-acl.c
   lib/qset-acl.c
+  lib/qsort.c
+  lib/qsort_r.c
   lib/readlink.c
   lib/readlinkat.c
   lib/root-uid.h
@@ -972,6 +982,7 @@
   m4/pselect.m4
   m4/pthread_sigmask.m4
   m4/putenv.m4
+  m4/qsort_r.m4
   m4/readlink.m4
   m4/readlinkat.m4
   m4/secure_getenv.m4

=== added file 'm4/qsort_r.m4'
--- m4/qsort_r.m4	1970-01-01 00:00:00 +0000
+++ m4/qsort_r.m4	2014-08-29 20:46:33 +0000
@@ -0,0 +1,48 @@
+dnl Reentrant sort function.
+
+dnl Copyright 2014 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+dnl Written by Paul Eggert.
+
+AC_DEFUN([gl_FUNC_QSORT_R],
+[
+  dnl Persuade glibc to declare qsort_r.
+  AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS])
+
+  AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
+
+  AC_CACHE_CHECK([for qsort_r signature], [gl_cv_qsort_r_signature],
+    [AC_LINK_IFELSE(
+       [AC_LANG_PROGRAM([[#include <stdlib.h>
+                          void qsort_r (void *, size_t, size_t,
+                                        int (*) (void const *, void const *,
+                                                 void *),
+                                        void *);
+                          void (*p) (void *, size_t, size_t,
+                                     int (*) (void const *, void const *,
+                                              void *),
+                                     void *) = qsort_r;
+                        ]])],
+       [gl_cv_qsort_r_signature=GNU],
+       [AC_LINK_IFELSE(
+          [AC_LANG_PROGRAM([[#include <stdlib.h>
+                             void qsort_r (void *, size_t, size_t, void *,
+                                           int (*) (void *,
+                                                    void const *,
+                                                    void const *));
+                             void (*p) (void *, size_t, size_t, void *,
+                                        int (*) (void *, void const *,
+                                                 void const *)) = qsort_r;
+                           ]])],
+          [gl_cv_qsort_r_signature=BSD],
+          [gl_cv_qsort_r_signature=no])])])
+
+  case $gl_cv_qsort_r_signature in
+    GNU) HAVE_QSORT_R=1;;
+    BSD) HAVE_QSORT_R=1 REPLACE_QSORT_R=1;;
+    *)   HAVE_QSORT_R=0 REPLACE_QSORT_R=1;;
+  esac
+])

=== modified file 'm4/stdlib_h.m4'
--- m4/stdlib_h.m4	2014-01-01 07:43:34 +0000
+++ m4/stdlib_h.m4	2014-08-29 20:46:33 +0000
@@ -55,6 +55,7 @@
   GNULIB_PTSNAME=0;       AC_SUBST([GNULIB_PTSNAME])
   GNULIB_PTSNAME_R=0;     AC_SUBST([GNULIB_PTSNAME_R])
   GNULIB_PUTENV=0;        AC_SUBST([GNULIB_PUTENV])
+  GNULIB_QSORT_R=0;       AC_SUBST([GNULIB_QSORT_R])
   GNULIB_RANDOM=0;        AC_SUBST([GNULIB_RANDOM])
   GNULIB_RANDOM_R=0;      AC_SUBST([GNULIB_RANDOM_R])
   GNULIB_REALLOC_POSIX=0; AC_SUBST([GNULIB_REALLOC_POSIX])
@@ -107,6 +108,7 @@
   REPLACE_PTSNAME=0;         AC_SUBST([REPLACE_PTSNAME])
   REPLACE_PTSNAME_R=0;       AC_SUBST([REPLACE_PTSNAME_R])
   REPLACE_PUTENV=0;          AC_SUBST([REPLACE_PUTENV])
+  REPLACE_QSORT_R=0;         AC_SUBST([REPLACE_QSORT_R])
   REPLACE_RANDOM_R=0;        AC_SUBST([REPLACE_RANDOM_R])
   REPLACE_REALLOC=0;         AC_SUBST([REPLACE_REALLOC])
   REPLACE_REALPATH=0;        AC_SUBST([REPLACE_REALPATH])

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2014-08-29 20:16:40 +0000
+++ src/ChangeLog	2014-08-29 20:46:33 +0000
@@ -1,5 +1,10 @@
 2014-08-29  Paul Eggert  <eggert@cs.ucla.edu>
 
+	Modularize qsort_r in Gnulib, to simplify Emacs proper.
+	* fns.c (sort_vector_predicate) [!HAVE_QSORT_R]: Remove.
+	(sort_vector_compare, sort_vector): Simplify by assuming GNU qsort_r,
+	since Gnulib now supplies it.
+
 	* sysdep.c (str_collate): Do not look at errno after towlower_l.
 	errno's value is not specified after towlower_l.  Instead, assume
 	that towlower_l returns its argument on failure, which is portable

=== modified file 'src/fns.c'
--- src/fns.c	2014-08-29 19:18:06 +0000
+++ src/fns.c	2014-08-29 20:46:33 +0000
@@ -1897,42 +1897,24 @@
   return merge (front, back, predicate);
 }
 
-/* Using GNU qsort_r, we can pass this as a parameter.  This also
-   exists on FreeBSD and Darwin/OSX, but with a different signature. */
-#ifndef HAVE_QSORT_R
-static Lisp_Object sort_vector_predicate;
-#endif
-
-/* Comparison function called by qsort.  */
+/* Comparison function called by qsort_r.  */
 
 static int
-#ifdef HAVE_QSORT_R
-#if defined (DARWIN_OS) || defined (__FreeBSD__)
-sort_vector_compare (void *arg, const void *p, const void *q)
-#elif defined (GNU_LINUX)
 sort_vector_compare (const void *p, const void *q, void *arg)
-#else /* neither darwin/bsd nor gnu/linux */
-#error "check how qsort_r comparison function works on your platform"
-#endif /* DARWIN_OS || __FreeBSD__ */
-#else /* not HAVE_QSORT_R */
-sort_vector_compare (const void *p, const void *q)
-#endif /* HAVE_QSORT_R */
 {
-  bool more, less;
-  Lisp_Object op, oq, vp, vq;
-#ifdef HAVE_QSORT_R
-  Lisp_Object sort_vector_predicate = *(Lisp_Object *) arg;
-#endif
-
-  op = *(Lisp_Object *) p;
-  oq = *(Lisp_Object *) q;
-  vp = XSAVE_OBJECT (op, 1);
-  vq = XSAVE_OBJECT (oq, 1);
+  Lisp_Object const *ap = p;
+  Lisp_Object const *aq = q;
+  Lisp_Object const *aarg = arg;
+  Lisp_Object op = *ap;
+  Lisp_Object oq = *aq;
+  Lisp_Object sort_vector_predicate = *aarg;
+  Lisp_Object vp = XSAVE_OBJECT (op, 1);
+  Lisp_Object vq = XSAVE_OBJECT (oq, 1);
 
   /* Use recorded element index as a secondary key to
      preserve original order.  Pretty ugly but works.  */
-  more = NILP (call2 (sort_vector_predicate, vp, vq));
-  less = NILP (call2 (sort_vector_predicate, vq, vp));
+  bool more = NILP (call2 (sort_vector_predicate, vp, vq));
+  bool less = NILP (call2 (sort_vector_predicate, vq, vp));
   return ((more && !less) ? 1
 	  : ((!more && less) ? -1
 	     : XSAVE_INTEGER (op, 0) - XSAVE_INTEGER (oq, 0)));
@@ -1950,23 +1932,11 @@
 
   if (len < 2)
     return vector;
-  /* Record original index of each element to make qsort stable.  */
+  /* Record original index of each element to make qsort_r stable.  */
   for (i = 0; i < len; i++)
     v[i] = make_save_int_obj (i, v[i]);
 
-  /* Setup predicate and sort.  */
-#ifdef HAVE_QSORT_R
-#if defined (DARWIN_OS) || defined (__FreeBSD__)
-  qsort_r (v, len, word_size, (void *) &predicate, sort_vector_compare);
-#elif defined (GNU_LINUX)
-  qsort_r (v, len, word_size, sort_vector_compare, (void *) &predicate);
-#else /* neither darwin/bsd nor gnu/linux */
-#error "check how qsort_r works on your platform"
-#endif /* DARWIN_OS || __FreeBSD__ */
-#else /* not HAVE_QSORT_R */
-  sort_vector_predicate = predicate;
-  qsort (v, len, word_size, sort_vector_compare);
-#endif /* HAVE_QSORT_R */
+  qsort_r (v, len, word_size, sort_vector_compare, &predicate);
 
   /* Discard indexes and restore original elements.  */
   for (i = 0; i < len; i++)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#18360: Modularize recent qsort_r additions
  2014-08-29 20:58 bug#18360: Modularize recent qsort_r additions Paul Eggert
@ 2014-08-30  7:29 ` Eli Zaretskii
  2014-08-30 13:44   ` Paul Eggert
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2014-08-30  7:29 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 18360

> Date: Fri, 29 Aug 2014 13:58:01 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> The recent changes to have Emacs use qsort_r look good except for the 
> forest of #ifdefs in Emacs proper.  Attached is a proposed patch to 
> migrate that stuff into Gnulib.  As usual most of this patch is 
> automatically generated; fns.c contains the crucial hand-generated 
> stuff.  I have not tested this on Microsoft Windows and so am posting it 
> here first to give Eli a heads-up.

Why is it a good idea to use qsort_r?  It's evidently not portable
enough, and its benefits for Emacs at this time are nil, AFAICT.

I say let's wait until we really need a reentrant sort function, and
introduce it then.  qsort is good enough for us for now.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#18360: Modularize recent qsort_r additions
  2014-08-30  7:29 ` Eli Zaretskii
@ 2014-08-30 13:44   ` Paul Eggert
  2014-08-30 14:15     ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Eggert @ 2014-08-30 13:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 18360

Eli Zaretskii wrote:
> qsort is good enough for us for now.

Unfortunately qsort doesn't work either, as discussed in Bug#18361.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#18360: Modularize recent qsort_r additions
  2014-08-30 13:44   ` Paul Eggert
@ 2014-08-30 14:15     ` Eli Zaretskii
  2014-08-30 16:39       ` Paul Eggert
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2014-08-30 14:15 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 18360

> Date: Sat, 30 Aug 2014 06:44:43 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 18360@debbugs.gnu.org
> 
> Eli Zaretskii wrote:
> > qsort is good enough for us for now.
> 
> Unfortunately qsort doesn't work either, as discussed in Bug#18361.

That is a different issue, and should not get in our way here.  For
the purposes of this bug, can we agree that qsort_r is not needed, and
the changes that use it should be reverted to use qsort instead?

We could then discuss separately what to do with qsort.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#18360: Modularize recent qsort_r additions
  2014-08-30 14:15     ` Eli Zaretskii
@ 2014-08-30 16:39       ` Paul Eggert
  2014-08-30 16:43         ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Eggert @ 2014-08-30 16:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 18360

Eli Zaretskii wrote:
> That is a different issue, and should not get in our way here.  For
> the purposes of this bug, can we agree that qsort_r is not needed, and
> the changes that use it should be reverted to use qsort instead?

Yes and no.  The previous version did not use qsort, so reverting the 
changes that use qsort_r will the drop the use of qsort (and qsort_r) 
entirely.

One could further *modify* the changes to stop using qsort_r, and to use 
qsort only.  But what would be the point?  qsort (and qsort_r) are 
mistakes here; we cannot use them.  Dropping the use of qsort_r would be 
like wiping the lipstick off a pig.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#18360: Modularize recent qsort_r additions
  2014-08-30 16:39       ` Paul Eggert
@ 2014-08-30 16:43         ` Eli Zaretskii
  2014-08-30 16:44           ` Paul Eggert
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2014-08-30 16:43 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 18360

> Date: Sat, 30 Aug 2014 09:39:32 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 18360@debbugs.gnu.org
> 
> Eli Zaretskii wrote:
> > That is a different issue, and should not get in our way here.  For
> > the purposes of this bug, can we agree that qsort_r is not needed, and
> > the changes that use it should be reverted to use qsort instead?
> 
> Yes and no.  The previous version did not use qsort, so reverting the 
> changes that use qsort_r will the drop the use of qsort (and qsort_r) 
> entirely.
> 
> One could further *modify* the changes to stop using qsort_r, and to use 
> qsort only.  But what would be the point?  qsort (and qsort_r) are 
> mistakes here; we cannot use them.  Dropping the use of qsort_r would be 
> like wiping the lipstick off a pig.

Then let's use a different sort routine, not qsort.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#18360: Modularize recent qsort_r additions
  2014-08-30 16:43         ` Eli Zaretskii
@ 2014-08-30 16:44           ` Paul Eggert
  2014-08-30 16:56             ` Paul Eggert
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Eggert @ 2014-08-30 16:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 18360

Eli Zaretskii wrote:
> Then let's use a different sort routine, not qsort.

Exactly my point!  I'll get on it.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#18360: Modularize recent qsort_r additions
  2014-08-30 16:44           ` Paul Eggert
@ 2014-08-30 16:56             ` Paul Eggert
  0 siblings, 0 replies; 8+ messages in thread
From: Paul Eggert @ 2014-08-30 16:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 18360-done

Paul Eggert wrote:
> Eli Zaretskii wrote:
>> Then let's use a different sort routine, not qsort.
>
> Exactly my point!  I'll get on it.

Oh, and I'll close this bug report, since we shouldn't use either qsort 
or qsort_r here.





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-08-30 16:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-29 20:58 bug#18360: Modularize recent qsort_r additions Paul Eggert
2014-08-30  7:29 ` Eli Zaretskii
2014-08-30 13:44   ` Paul Eggert
2014-08-30 14:15     ` Eli Zaretskii
2014-08-30 16:39       ` Paul Eggert
2014-08-30 16:43         ` Eli Zaretskii
2014-08-30 16:44           ` Paul Eggert
2014-08-30 16:56             ` Paul Eggert

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).