[Top][All Lists]
[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) ?