From: Paul Eggert <eggert@cs.ucla.edu>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
Subject: Re: bookkeeping to prepare for a 64-bit EMACS_INT on 32-bit hosts
Date: Fri, 29 Apr 2011 18:37:38 -0700 [thread overview]
Message-ID: <4DBB67E2.1040202@cs.ucla.edu> (raw)
In-Reply-To: <jwvy62thxsh.fsf-monnier+emacs@gnu.org>
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.
next prev parent reply other threads:[~2011-04-30 1:37 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-29 8:08 bookkeeping to prepare for a 64-bit EMACS_INT on 32-bit hosts Paul Eggert
2011-04-29 8:49 ` Eli Zaretskii
2011-04-29 9:06 ` Paul Eggert
2011-04-29 9:17 ` Eli Zaretskii
2011-04-29 16:04 ` Stefan Monnier
2011-04-29 17:10 ` Eli Zaretskii
2011-04-29 17:32 ` Lars Magne Ingebrigtsen
2011-04-29 17:45 ` Paul Eggert
2011-04-29 18:50 ` David De La Harpe Golden
2011-04-30 5:00 ` Stephen J. Turnbull
2011-04-30 1:37 ` Paul Eggert [this message]
2011-05-02 14:46 ` Stefan Monnier
2011-05-02 16:09 ` Paul Eggert
2011-05-02 18:12 ` Stefan Monnier
2011-05-02 19:23 ` Paul Eggert
2011-05-02 19:49 ` Stefan Monnier
2011-04-30 6:54 ` Paul Eggert
2011-04-30 7:30 ` Eli Zaretskii
2011-04-29 8:56 ` support " Paul Eggert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DBB67E2.1040202@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=eliz@gnu.org \
--cc=emacs-devel@gnu.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.