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.

11 files changed, 108 insertions, 72 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index 81b23bccc8b..73fc64f37c5 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,20 @@
+2013-07-19  Paul Eggert  <eggert@cs.ucla.edu>
+
+        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  <eggert@cs.ucla.edu>
 
         * keyboard.c: Try to fix typos in previous change.
diff --git a/src/alloc.c b/src/alloc.c
index 39f6a82b138..11245741bd5 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -3342,62 +3342,81 @@ verify (((SAVE_INTEGER | SAVE_POINTER | SAVE_FUNCPOINTER | SAVE_OBJECT)
          >> 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.  */
+/* Return Lisp_Save_Value objects for the various combinations
+   that callers need.  */
 
 Lisp_Object
-make_save_value (enum Lisp_Save_Type save_type, ...)
+make_save_int_int_int (ptrdiff_t a, ptrdiff_t b, ptrdiff_t c)
 {
-  va_list ap;
-  int i;
   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;
+}
 
-  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;
+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;
+}
 
-      case SAVE_OBJECT:
-        p->data[i].object = va_arg (ap, Lisp_Object);
-        break;
+#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 = a;
+  return val;
+}
+#endif
 
-      default:
-        emacs_abort ();
-      }
+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;
+}
 
-  va_end (ap);
+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;
 }
 
-/* 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.  */
+/* Return a Lisp_Save_Value object that represents an array A
+   of N Lisp objects.  */
 
 Lisp_Object
-make_save_pointer (void *pointer)
+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_POINTER;
-  p->data[0].pointer = pointer;
+  p->save_type = SAVE_TYPE_MEMORY;
+  p->data[0].pointer = a;
+  p->data[1].integer = n;
   return val;
 }
 
diff --git a/src/editfns.c b/src/editfns.c
index a4dea203a22..50bde90788d 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -838,9 +838,8 @@ This function does not move point.  */)
 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)
diff --git a/src/fileio.c b/src/fileio.c
index 5fe359d58bb..a19fcd9f663 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -4215,8 +4215,7 @@ by calling `format-decode', which see.  */)
                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))
diff --git a/src/font.c b/src/font.c
index 80b4b76c4e4..124d5f9bd9e 100644
--- a/src/font.c
+++ b/src/font.c
@@ -1861,7 +1861,7 @@ otf_open (Lisp_Object file)
   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;
diff --git a/src/ftfont.c b/src/ftfont.c
index 7c9534d5ae7..10090cb3bda 100644
--- a/src/ftfont.c
+++ b/src/ftfont.c
@@ -393,7 +393,7 @@ ftfont_lookup_cache (Lisp_Object key, enum ftfont_cache_for cache_for)
       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);
     }
diff --git a/src/keymap.c b/src/keymap.c
index e1268c8a06c..d13a6274347 100644
--- a/src/keymap.c
+++ b/src/keymap.c
@@ -617,8 +617,8 @@ map_keymap_internal (Lisp_Object map,
         }
       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;
diff --git a/src/lisp.h b/src/lisp.h
index 518de9db0ff..254ead231b9 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -441,8 +441,7 @@ enum Lisp_Fwd_Type
    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 @@ enum Lisp_Save_Type
 
    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 @@ struct Lisp_Save_Value
     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 bool abort_on_gc;
 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 @@ extern void *record_xmalloc (size_t);
       {                                                        \
         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_);         \
       }                                                        \
diff --git a/src/nsterm.m b/src/nsterm.m
index c91e68f37a9..f3c35e95bfe 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3777,7 +3777,7 @@ ns_set_vertical_scroll_bar (struct window *window,
         }
 
       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
     {
diff --git a/src/w32fns.c b/src/w32fns.c
index 5d9200bdd7b..675b716f3b0 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -4916,7 +4916,7 @@ w32_monitor_enum (HMONITOR monitor, HDC hdc, RECT *rcMonitor, LPARAM dwData)
 {
   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;
 }
diff --git a/src/xmenu.c b/src/xmenu.c
index 1151dea440e..6c0e3dd78a6 100644
--- a/src/xmenu.c
+++ b/src/xmenu.c
@@ -2465,8 +2465,7 @@ xmenu_show (FRAME_PTR f, int x, int y, bool for_click, bool keymaps,
   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.  */