emacs-devel
[Top][All Lists]
Advanced

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

Re: [patch] mem_node shrink


From: Richard Stallman
Subject: Re: [patch] mem_node shrink
Date: Wed, 21 Nov 2007 21:26:35 -0500

The idea looks like an improvement.  I think there needs to be a comment
explaining that the widths of these fields are supposed to add up to
the same as an EMACS_INT.

And is EMACS_INT the right thing?  EMACS_INT is `long' in some cases.
Should it be plain `int' instead?  Should it be a type that's as wide
as a pointer or as size_t?

Should the size value be measured in units of Lisp_Object instead
of bytes?

    +  if (size > MOST_POSITIVE_FIXNUM)
    +    abort ();
    +#endif

That's not reliably the correct test.  It happens to be right, at
present, because the width of the field is BITS_PER_EMACS_INT - 4, and
it is unsigned, so it has the same number of bits as a positive Lisp
integer.  But that is just coincidence.

So I think MOST_POSITIVE_FIXNUM should be replaced with something
guaranteed to be right.  Define a constant to serve as the width of
that field, and use the same constant here in something like -((-1) <<
MEM_NODE_SIZE_WIDTH).

         /* Can't handle zero size regions in the red-black tree.  */
    -    mem_insert (value, (char *) value + max (size, 1), MEM_TYPE_NON_LISP);
    +    mem_insert (value, max (size, 1), MEM_TYPE_NON_LISP);

Wouldn't it be cleaner for mem_insert to do  max (..., 1) ?




reply via email to

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