]> git.eshelyaron.com Git - emacs.git/commitdiff
Fix obscure porting bug with varargs functions.
authorPaul Eggert <eggert@cs.ucla.edu>
Fri, 19 Jul 2013 01:24:35 +0000 (18:24 -0700)
committerPaul Eggert <eggert@cs.ucla.edu>
Fri, 19 Jul 2013 01:24:35 +0000 (18:24 -0700)
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.

src/ChangeLog
src/alloc.c
src/editfns.c
src/fileio.c
src/font.c
src/ftfont.c
src/keymap.c
src/lisp.h
src/nsterm.m
src/w32fns.c
src/xmenu.c

index 81b23bccc8b68d42061ff46fdbb638c45cc5dadf..73fc64f37c51281d923d6fb9f05cce1ae3d02fde 100644 (file)
@@ -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.
index 39f6a82b138e01fd8f12b4a2ab29f356c4c841c0..11245741bd591838376f64b6610a8ec868f92001 100644 (file)
@@ -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;
 }
 
index a4dea203a22981d00744478ae36ee86ff828614f..50bde90788d9ad761fef2bc39f7f6f926cd1a24a 100644 (file)
@@ -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)
index 5fe359d58bbc36b8a6a1683e3e4e1c2e8bd4aed4..a19fcd9f66380a33364a6607589d063caa192c70 100644 (file)
@@ -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))
index 80b4b76c4e486d8359c58a2cc394539095717e67..124d5f9bd9e241c28ae46e585bfffe4363d947b7 100644 (file)
@@ -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;
index 7c9534d5ae76734f9e812cfc7f35dc0d6550322d..10090cb3bda07f444a48a3b11815027db01133d1 100644 (file)
@@ -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);
     }
index e1268c8a06c27d1e43abce9dbac1f77310e4c53a..d13a6274347ae93ac3e5bcdcfbb405710b2a5e97 100644 (file)
@@ -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;
index 518de9db0ffc30b7a22e332ee9261636c448962b..254ead231b9c7c932ecd2e900d39d47b651eaa7c 100644 (file)
@@ -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_);         \
       }                                                               \
index c91e68f37a9ad1a8607fcf4e24b98ca2b74b1373..f3c35e95bfee86a39b7b9a306e86c573c23d29cd 100644 (file)
@@ -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
     {
index 5d9200bdd7bb06f752e9aadf03aec777b0c541e7..675b716f3b06e0adde96b1b43ab18d1110f0f9a4 100644 (file)
@@ -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;
 }
index 1151dea440e68bb00905b438f3b5365d946c036b..6c0e3dd78a689c60ffc2e8e55a01a5168ec7bd12 100644 (file)
@@ -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.  */