64-unclean code in eval.c

[Ben Wing]

Stephen J. Turnbull wrote:
> This code in eval.c at l. 6016 gets flagged as 64-bit unsafe
> (specifically on amd64 pointers and EMACS_INTs are 64 bits, while ints
> are 32 bits).  In fact, in C++ it's an error.  Anybody know what
> should be happening here?  I would assume that all C integers that are
> visible to Lisp are EMACS_INTs, but if there not changing the
> declaration of val to EMACS_INT below would be a disaster.
>
> static Lisp_Object
> restore_int (Lisp_Object cons)
> {
>   Lisp_Object opaque = XCAR (cons);
>   Lisp_Object lval = XCDR (cons);
>   int *addr = (int *) get_opaque_ptr (opaque);
>   int val;
>
>   if (INTP (lval))
>     val = XINT (lval);
>   else
>     {
>       val = (int) get_opaque_ptr (lval);
>       free_opaque_ptr (lval);
>     }
>
>   *addr = val;
>   free_opaque_ptr (opaque);
>   free_cons (cons);
>   return Qnil;
> }
>
>   
i'm staring at this code and i don't see what's wrong even on 64-bit 
platforms.  i'm pretty sure i wrote this code.  it is used for saving 
and restoring internal C variables of type `int' and it tries to put it 
into a Lisp int but it's careful to use a opaque pointer in the rare 
instance where the value won't fit into a Lisp int.  the code is careful 
with its casting so i don't see what the problem is.

if the problem is this line:

      val = (int) get_opaque_ptr (lval);


then something is too smart for its own good.

i would not recommend Aidan's patch because i think it's too obscure.  
instead, put in a comment to the effect that we know the value stored in 
the void * is a crypto-int since we put an int into it in the first 
place, and i'd use more clever casts to get rid of the problem.  for 
example, cast to (long) and then to (int).

btw how can this be an error under C++?  when i used to do devwork i 
always ran a C++ build as part of my tests, and unless something changed 
recently, this code has been around for long before then.

and if it is indeed a problem under C++, use this:

      val = reinterpret_cast<int> get_opaque_ptr (lval);


that's the C++ way of saying "abandon hope all ye who enter here" :)

ben