Re: bookkeeping to prepare for a 64-bit EMACS_INT on 32-bit hosts

[Paul Eggert]

On 04/29/11 09:04, Stefan Monnier wrote:

> why use EMACS_INTPTR rather than intptr_t?

An excess of caution. :-)  I'll change it to intptr_t.

>>>>> >>> > -  if (data != NULL && data == (void*) XHASH (QCdbus_session_bus))
>>>>> >>> > +  if (data != NULL && data == (void *) XPNTR (QCdbus_session_bus))
> ...
> I.e. XPNTR should never be used on an INTEGERP value.

OK, thanks.  Since busses are always symbols, I'll change that to:

  if (SYMBOLP (QCdbus_session_bus) && XSYMBOL (QCdbus_session_bus) == data)

and similarly change the other two XPNTRs to SYMBOLP-guarded XSYMBOLs.

With this change, there shouldn't be a need to cast to void *, right?
(A cast would be needed if Emacs were intended to be compilable by
a C++ compiler, but I assume that's not a goal.)

>> > -    docstring = make_number (XHASH (function));
>> > +    docstring = make_number (XPNTR (function));
> You lost me here.  make_number doesn't take a pointer as argument.
> Even tho it's called "hash" it should not lose any information, so XHASH
> is the right thing to use here, AFAICT.

But 'function' is a Lisp_Object, so it's already a tagged pointer
that's possibly shifted.  make_number will tag it and possibly shift
it again, which can lose info about the pointer; and this means
purecopy's hash-consing could mess up.

In other words, the XHASH can cause a bug even on an ordinary 32-bit host.

XPNTR can't lose any information about the actual pointer, since
'function' is guaranteed to be a symbol here.  That's why this code is
different from the dbus code mentioned above.

>> > -      /* The EMACS_INT cast avoids a warning. */
>> > +      EMACS_INTPTR ii = i;
>> > +      gpointer gi = (gpointer) ii;
> Is there a particular reason why you use an intermediate var rather than use 
> the
> more concise "(gpointer) (EMACS_INTPTR) i"?

To avoid a cast.  If you prefer conciseness to avoiding these casts, I
can easily change these to the more-concise form.

Thanks for the careful review.