Re: [patch] mem_node shrink

[Richard Stallman <rms@gnu.org>]

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) ?