[Emacs-diffs] trunk r113453: Fix obscure porting bug with varargs functions.

[Paul Eggert]

------------------------------------------------------------
revno: 113453
revision-id: address@hidden
parent: address@hidden
committer: Paul Eggert <address@hidden>
branch nick: trunk
timestamp: Thu 2013-07-18 18:24:35 -0700
message:
  Fix obscure porting bug with varargs functions.
  
  The code assumed that int is treated like ptrdiff_t in a vararg
  function, which is not a portable assumption.  There was a similar
  -- though these days less likely -- porting problem with various
  assumptions that pointers of different types all smell the same as
  far as vararg functions is conserved.  To make this problem less
  likely in the future, redo the API to use varargs functions.
  * alloc.c (make_save_value): Remove this vararg function.
  All uses changed to ...
  (make_save_int_int_int, make_save_obj_obj_obj_obj)
  (make_save_ptr_int, make_save_funcptr_ptr_obj, make_save_memory):
  New functions.
  (make_save_ptr): Rename from make_save_pointer, for consistency with
  the above.  Define only on platforms that need it.  All uses changed.
modified:
  src/ChangeLog                  changelog-20091113204419-o5vbwnq5f7feedwu-1438
  src/alloc.c                    alloc.c-20091113204419-o5vbwnq5f7feedwu-252
  src/editfns.c                  editfns.c-20091113204419-o5vbwnq5f7feedwu-255
  src/fileio.c                   fileio.c-20091113204419-o5vbwnq5f7feedwu-210
  src/font.c                     font.c-20091113204419-o5vbwnq5f7feedwu-8540
  src/ftfont.c                   ftfont.c-20091113204419-o5vbwnq5f7feedwu-8542
  src/keymap.c                   keymap.c-20091113204419-o5vbwnq5f7feedwu-219
  src/lisp.h                     lisp.h-20091113204419-o5vbwnq5f7feedwu-253
  src/nsterm.m                   nsterm.m-20091113204419-o5vbwnq5f7feedwu-8747
  src/w32fns.c                   w32fns.c-20091113204419-o5vbwnq5f7feedwu-945
  src/xmenu.c                    xmenu.c-20091113204419-o5vbwnq5f7feedwu-161

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2013-07-19 00:38:19 +0000
+++ b/src/ChangeLog     2013-07-19 01:24:35 +0000
@@ -1,3 +1,20 @@
+2013-07-19  Paul Eggert  <address@hidden>
+
+       Fix obscure porting bug with varargs functions.
+       The code assumed that int is treated like ptrdiff_t in a vararg
+       function, which is not a portable assumption.  There was a similar
+       -- though these days less likely -- porting problem with various
+       assumptions that pointers of different types all smell the same as
+       far as vararg functions is conserved.  To make this problem less
+       likely in the future, redo the API to use varargs functions.
+       * alloc.c (make_save_value): Remove this vararg function.
+       All uses changed to ...
+       (make_save_int_int_int, make_save_obj_obj_obj_obj)
+       (make_save_ptr_int, make_save_funcptr_ptr_obj, make_save_memory):
+       New functions.
+       (make_save_ptr): Rename from make_save_pointer, for consistency with
+       the above.  Define only on platforms that need it.  All uses changed.
+
 2013-07-18  Paul Eggert  <address@hidden>
 
        * keyboard.c: Try to fix typos in previous change.

=== modified file 'src/alloc.c'
--- a/src/alloc.c       2013-07-16 21:35:45 +0000
+++ b/src/alloc.c       2013-07-19 01:24:35 +0000
@@ -3342,62 +3342,81 @@
         >> SAVE_SLOT_BITS)
        == 0);
 
-/* Return a Lisp_Save_Value object with the data saved according to
-   DATA_TYPE.  DATA_TYPE should be one of SAVE_TYPE_INT_INT, etc.  */
-
-Lisp_Object
-make_save_value (enum Lisp_Save_Type save_type, ...)
-{
-  va_list ap;
-  int i;
-  Lisp_Object val = allocate_misc (Lisp_Misc_Save_Value);
-  struct Lisp_Save_Value *p = XSAVE_VALUE (val);
-
-  eassert (0 < save_type
-          && (save_type < 1 << (SAVE_TYPE_BITS - 1)
-              || save_type == SAVE_TYPE_MEMORY));
-  p->save_type = save_type;
-  va_start (ap, save_type);
-  save_type &= ~ (1 << (SAVE_TYPE_BITS - 1));
-
-  for (i = 0; save_type; i++, save_type >>= SAVE_SLOT_BITS)
-    switch (save_type & ((1 << SAVE_SLOT_BITS) - 1))
-      {
-      case SAVE_POINTER:
-       p->data[i].pointer = va_arg (ap, void *);
-       break;
-
-      case SAVE_FUNCPOINTER:
-       p->data[i].funcpointer = va_arg (ap, voidfuncptr);
-       break;
-
-      case SAVE_INTEGER:
-       p->data[i].integer = va_arg (ap, ptrdiff_t);
-       break;
-
-      case SAVE_OBJECT:
-       p->data[i].object = va_arg (ap, Lisp_Object);
-       break;
-
-      default:
-       emacs_abort ();
-      }
-
-  va_end (ap);
-  return val;
-}
-
-/* Save just one C pointer.  record_unwind_protect_ptr is simpler and
-   faster than combining this with record_unwind_protect, but
-   occasionally this function is useful for other reasons.  */
-
-Lisp_Object
-make_save_pointer (void *pointer)
+/* Return Lisp_Save_Value objects for the various combinations
+   that callers need.  */
+
+Lisp_Object
+make_save_int_int_int (ptrdiff_t a, ptrdiff_t b, ptrdiff_t c)
+{
+  Lisp_Object val = allocate_misc (Lisp_Misc_Save_Value);
+  struct Lisp_Save_Value *p = XSAVE_VALUE (val);
+  p->save_type = SAVE_TYPE_INT_INT_INT;
+  p->data[0].integer = a;
+  p->data[1].integer = b;
+  p->data[2].integer = c;
+  return val;
+}
+
+Lisp_Object
+make_save_obj_obj_obj_obj (Lisp_Object a, Lisp_Object b, Lisp_Object c,
+                          Lisp_Object d)
+{
+  Lisp_Object val = allocate_misc (Lisp_Misc_Save_Value);
+  struct Lisp_Save_Value *p = XSAVE_VALUE (val);
+  p->save_type = SAVE_TYPE_OBJ_OBJ_OBJ_OBJ;
+  p->data[0].object = a;
+  p->data[1].object = b;
+  p->data[2].object = c;
+  p->data[3].object = d;
+  return val;
+}
+
+#if defined HAVE_NS || defined DOS_NT
+Lisp_Object
+make_save_ptr (void *a)
 {
   Lisp_Object val = allocate_misc (Lisp_Misc_Save_Value);
   struct Lisp_Save_Value *p = XSAVE_VALUE (val);
   p->save_type = SAVE_POINTER;
-  p->data[0].pointer = pointer;
+  p->data[0].pointer = a;
+  return val;
+}
+#endif
+
+Lisp_Object
+make_save_ptr_int (void *a, ptrdiff_t b)
+{
+  Lisp_Object val = allocate_misc (Lisp_Misc_Save_Value);
+  struct Lisp_Save_Value *p = XSAVE_VALUE (val);
+  p->save_type = SAVE_TYPE_PTR_INT;
+  p->data[0].pointer = a;
+  p->data[1].integer = b;
+  return val;
+}
+
+Lisp_Object
+make_save_funcptr_ptr_obj (void (*a) (void), void *b, Lisp_Object c)
+{
+  Lisp_Object val = allocate_misc (Lisp_Misc_Save_Value);
+  struct Lisp_Save_Value *p = XSAVE_VALUE (val);
+  p->save_type = SAVE_TYPE_FUNCPTR_PTR_OBJ;
+  p->data[0].funcpointer = a;
+  p->data[1].pointer = b;
+  p->data[2].object = c;
+  return val;
+}
+
+/* Return a Lisp_Save_Value object that represents an array A
+   of N Lisp objects.  */
+
+Lisp_Object
+make_save_memory (Lisp_Object *a, ptrdiff_t n)
+{
+  Lisp_Object val = allocate_misc (Lisp_Misc_Save_Value);
+  struct Lisp_Save_Value *p = XSAVE_VALUE (val);
+  p->save_type = SAVE_TYPE_MEMORY;
+  p->data[0].pointer = a;
+  p->data[1].integer = n;
   return val;
 }
 

=== modified file 'src/editfns.c'
--- a/src/editfns.c     2013-07-17 17:24:54 +0000
+++ b/src/editfns.c     2013-07-19 01:24:35 +0000
@@ -838,9 +838,8 @@
 Lisp_Object
 save_excursion_save (void)
 {
-  return make_save_value
-    (SAVE_TYPE_OBJ_OBJ_OBJ_OBJ,
-     Fpoint_marker (),
+  return make_save_obj_obj_obj_obj
+    (Fpoint_marker (),
      /* Do not copy the mark if it points to nowhere.  */
      (XMARKER (BVAR (current_buffer, mark))->buffer
       ? Fcopy_marker (BVAR (current_buffer, mark), Qnil)

=== modified file 'src/fileio.c'
--- a/src/fileio.c      2013-07-18 02:12:59 +0000
+++ b/src/fileio.c      2013-07-19 01:24:35 +0000
@@ -4215,8 +4215,7 @@
               to be signaled after decoding the text we read.  */
            nbytes = internal_condition_case_1
              (read_non_regular,
-              make_save_value (SAVE_TYPE_INT_INT_INT, (ptrdiff_t) fd,
-                               inserted, trytry),
+              make_save_int_int_int (fd, inserted, trytry),
               Qerror, read_non_regular_quit);
 
            if (NILP (nbytes))

=== modified file 'src/font.c'
--- a/src/font.c        2013-07-16 06:39:49 +0000
+++ b/src/font.c        2013-07-19 01:24:35 +0000
@@ -1861,7 +1861,7 @@
   else
     {
       otf = STRINGP (file) ? OTF_open (SSDATA (file)) : NULL;
-      val = make_save_pointer (otf);
+      val = make_save_ptr (otf);
       otf_list = Fcons (Fcons (file, val), otf_list);
     }
   return otf;

=== modified file 'src/ftfont.c'
--- a/src/ftfont.c      2013-07-16 06:39:49 +0000
+++ b/src/ftfont.c      2013-07-19 01:24:35 +0000
@@ -393,7 +393,7 @@
       cache_data = xmalloc (sizeof *cache_data);
       cache_data->ft_face = NULL;
       cache_data->fc_charset = NULL;
-      val = make_save_value (SAVE_TYPE_PTR_INT, cache_data, 0);
+      val = make_save_ptr_int (cache_data, 0);
       cache = Fcons (Qnil, val);
       Fputhash (key, cache, ft_face_cache);
     }

=== modified file 'src/keymap.c'
--- a/src/keymap.c      2013-07-16 06:39:49 +0000
+++ b/src/keymap.c      2013-07-19 01:24:35 +0000
@@ -617,8 +617,8 @@
        }
       else if (CHAR_TABLE_P (binding))
        map_char_table (map_keymap_char_table_item, Qnil, binding,
-                       make_save_value (SAVE_TYPE_FUNCPTR_PTR_OBJ,
-                                        (voidfuncptr) fun, data, args));
+                       make_save_funcptr_ptr_obj ((voidfuncptr) fun, data,
+                                                  args));
     }
   UNGCPRO;
   return tail;

=== modified file 'src/lisp.h'
--- a/src/lisp.h        2013-07-18 02:12:59 +0000
+++ b/src/lisp.h        2013-07-19 01:24:35 +0000
@@ -441,8 +441,7 @@
    displayed to users.  These are Lisp_Save_Value, a Lisp_Misc
    subtype; and PVEC_OTHER, a kind of vectorlike object.  The former
    is suitable for temporarily stashing away pointers and integers in
-   a Lisp object (see the existing uses of make_save_value and
-   XSAVE_VALUE).  The latter is useful for vector-like Lisp objects
+   a Lisp object.  The latter is useful for vector-like Lisp objects
    that need to be used as part of other objects, but which are never
    shown to users or Lisp code (search for PVEC_OTHER in xterm.c for
    an example).
@@ -1815,30 +1814,26 @@
 
    This is mostly used to package C integers and pointers to call
    record_unwind_protect when two or more values need to be saved.
-   make_save_value lets you pack up to SAVE_VALUE_SLOTS integers, pointers,
-   function pointers or Lisp_Objects and conveniently get them back
-   with XSAVE_INTEGER, XSAVE_POINTER, XSAVE_FUNCPOINTER, and
-   XSAVE_OBJECT macros:
+   For example:
 
    ...
      struct my_data *md = get_my_data ();
-     Lisp_Object my_object = get_my_object ();
-     record_unwind_protect
-       (my_unwind, make_save_value (SAVE_TYPE_PTR_OBJ, md, my_object));
+     ptrdiff_t mi = get_my_integer ();
+     record_unwind_protect (my_unwind, make_save_ptr_int (md, mi));
    ...
 
    Lisp_Object my_unwind (Lisp_Object arg)
    {
      struct my_data *md = XSAVE_POINTER (arg, 0);
-     Lisp_Object my_object = XSAVE_OBJECT (arg, 1);
+     ptrdiff_t mi = XSAVE_INTEGER (arg, 1);
      ...
    }
 
    If ENABLE_CHECKING is in effect, XSAVE_xxx macros do type checking of the
    saved objects and raise eassert if type of the saved object doesn't match
    the type which is extracted.  In the example above, XSAVE_INTEGER (arg, 2)
-   or XSAVE_OBJECT (arg, 0) are wrong because nothing was saved in slot 2 and
-   Lisp_Object was saved in slot 1 of ARG.  */
+   and XSAVE_OBJECT (arg, 0) are wrong because nothing was saved in slot 2 and
+   slot 0 is a pointer.  */
 
 typedef void (*voidfuncptr) (void);
 
@@ -1848,12 +1843,13 @@
     unsigned gcmarkbit : 1;
     int spacer : 32 - (16 + 1 + SAVE_TYPE_BITS);
 
-    /* DATA[N] may hold up to SAVE_VALUE_SLOTS entries.  The type of
-       V's Ith entry is given by save_type (V, I).  E.g., if save_type
-       (V, 3) == SAVE_INTEGER, V->data[3].integer is in use.
+    /* V->data may hold up to SAVE_VALUE_SLOTS entries.  The type of
+       V's data entries are determined by V->save_type.  E.g., if
+       V->save_type == SAVE_TYPE_PTR_OBJ, V->data[0] is a pointer,
+       V->data[1] is an integer, and V's other data entries are unused.
 
-       If SAVE_TYPE == SAVE_TYPE_MEMORY, DATA[0].pointer is the address of
-       a memory area containing DATA[1].integer potential Lisp_Objects.  */
+       If V->save_type == SAVE_TYPE_MEMORY, V->data[0].pointer is the address 
of
+       a memory area containing V->data[1].integer potential Lisp_Objects.  */
     ENUM_BF (Lisp_Save_Type) save_type : SAVE_TYPE_BITS;
     union {
       void *pointer;
@@ -3580,8 +3576,15 @@
 extern Lisp_Object make_float (double);
 extern void display_malloc_warning (void);
 extern ptrdiff_t inhibit_garbage_collection (void);
-extern Lisp_Object make_save_value (enum Lisp_Save_Type, ...);
-extern Lisp_Object make_save_pointer (void *);
+extern Lisp_Object make_save_int_int_int (ptrdiff_t, ptrdiff_t, ptrdiff_t);
+extern Lisp_Object make_save_obj_obj_obj_obj (Lisp_Object, Lisp_Object,
+                                             Lisp_Object, Lisp_Object);
+extern Lisp_Object make_save_ptr (void *);
+extern Lisp_Object make_save_ptr_int (void *, ptrdiff_t);
+extern Lisp_Object make_save_ptr_ptr (void *, void *);
+extern Lisp_Object make_save_funcptr_ptr_obj (void (*) (void), void *,
+                                             Lisp_Object);
+extern Lisp_Object make_save_memory (Lisp_Object *, ptrdiff_t);
 extern void free_save_value (Lisp_Object);
 extern Lisp_Object build_overlay (Lisp_Object, Lisp_Object, Lisp_Object);
 extern void free_marker (Lisp_Object);
@@ -4314,7 +4317,7 @@
       {                                                               \
        Lisp_Object arg_;                                      \
        buf = xmalloc ((nelt) * word_size);                    \
-       arg_ = make_save_value (SAVE_TYPE_MEMORY, buf, nelt);  \
+       arg_ = make_save_memory (buf, nelt);                   \
        sa_must_free = 1;                                      \
        record_unwind_protect (free_save_value, arg_);         \
       }                                                               \

=== modified file 'src/nsterm.m'
--- a/src/nsterm.m      2013-07-16 11:41:06 +0000
+++ b/src/nsterm.m      2013-07-19 01:24:35 +0000
@@ -3777,7 +3777,7 @@
         }
 
       bar = [[EmacsScroller alloc] initFrame: r window: win];
-      wset_vertical_scroll_bar (window, make_save_pointer (bar));
+      wset_vertical_scroll_bar (window, make_save_ptr (bar));
     }
   else
     {

=== modified file 'src/w32fns.c'
--- a/src/w32fns.c      2013-07-16 23:29:05 +0000
+++ b/src/w32fns.c      2013-07-19 01:24:35 +0000
@@ -4916,7 +4916,7 @@
 {
   Lisp_Object *monitor_list = (Lisp_Object *) dwData;
 
-  *monitor_list = Fcons (make_save_pointer (monitor), *monitor_list);
+  *monitor_list = Fcons (make_save_ptr (monitor), *monitor_list);
 
   return TRUE;
 }

=== modified file 'src/xmenu.c'
--- a/src/xmenu.c       2013-07-16 21:35:45 +0000
+++ b/src/xmenu.c       2013-07-19 01:24:35 +0000
@@ -2465,8 +2465,7 @@
   XMenuActivateSetWaitFunction (x_menu_wait_for_event, FRAME_X_DISPLAY (f));
 #endif
 
-  record_unwind_protect (pop_down_menu,
-                        make_save_value (SAVE_TYPE_PTR_PTR, f, menu));
+  record_unwind_protect (pop_down_menu, make_save_ptr_ptr (f, menu));
 
   /* Help display under X won't work because XMenuActivate contains
      a loop that doesn't give Emacs a chance to process it.  */