[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Paul Eggert
Subject: Re: bookkeeping to prepare for a 64-bit EMACS_INT on 32-bit hosts
Date: Fri, 29 Apr 2011 18:37:38 -0700
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20110223 Thunderbird/3.1.8

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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]