Intervals crash

Intervals crash

[Chong Yidong]

I'm getting a crash due to your recent EMACS_UINT change.

  emacs -Q
  C-x C-f tr TAB => crash

  #0  abort () at emacs.c:423
  #1  0x0824356b in delete_interval (i=0x84a8cbc) at intervals.c:1269
  #2  0x08243a36 in merge_interval_right (i=0x84a8cbc) at intervals.c:1474
  #3  0x08242d93 in adjust_intervals_for_insertion (tree=0x84b7e7c, position=12,
      length=8) at intervals.c:1027
  #4  0x0824396d in offset_intervals (buffer=0x85de228, start=12, length=8)
      at intervals.c:1436
  #5  0x08191207 in insert_from_string_1 (string=141199641, pos=0, pos_byte=0,
      nchars=8, nbytes=8, inherit=0, before_markers=0) at insdel.c:1087
  #6  0x08190ddf in insert_from_string (string=141199641, pos=0, pos_byte=0,
      length=8, length_byte=8, inherit=0) at insdel.c:986
  ...
  Lisp Backtrace:
  "completion--replace" (0xbfffd654)
  "completion--do-completion" (0xbfffd944)
  "minibuffer-complete" (0xbfffdc54)
  ...

The LENGTH macro in intervals.h:114 has to be able to return a negative
number.  This is probably worth reverting, until you come up with a
proper fix.

Re: Intervals crash

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

> I'm getting a crash due to your recent EMACS_UINT change.
>
> The LENGTH macro in intervals.h:114 has to be able to return a negative
> number.

I converted the EMACS_UINTs to EMACS_INT; this fixes the crash, and I
think it is probably OK.

Re: Intervals crash

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Thu, 23 Sep 2010 14:52:48 -0400
> Cc: emacs-devel@gnu.org
> 
> Chong Yidong <cyd@stupidchicken.com> writes:
> 
> > I'm getting a crash due to your recent EMACS_UINT change.
> >
> > The LENGTH macro in intervals.h:114 has to be able to return a negative
> > number.
> 
> I converted the EMACS_UINTs to EMACS_INT; this fixes the crash, and I
> think it is probably OK.

You didn't say you are going to work on this, so we both worked in
parallel.

Re: Intervals crash

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> > I'm getting a crash due to your recent EMACS_UINT change.
>> >
>> > The LENGTH macro in intervals.h:114 has to be able to return a negative
>> > number.
>>
>> I converted the EMACS_UINTs to EMACS_INT; this fixes the crash, and I
>> think it is probably OK.
>
> You didn't say you are going to work on this, so we both worked in
> parallel.

Sorry about that.  I was in the middle of performing a merge from the
emacs-23 branch, and the crash was interrupting testing.  I guess I'll
wait before doing the merge, in case you have any more changes to make.

Re: Intervals crash

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 23 Sep 2010 15:09:33 -0400
> 
> > You didn't say you are going to work on this, so we both worked in
> > parallel.
> 
> Sorry about that.

No sweat, we made almost exactly the same change ;-)

> I guess I'll wait before doing the merge, in case you have any more
> changes to make.

It's time for a cease-fire for today.  More damage tomorrow ;-)

So you can go ahead with the merge.

Re: Intervals crash

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Thu, 23 Sep 2010 14:23:24 -0400
> Cc: emacs-devel@gnu.org
> 
> I'm getting a crash due to your recent EMACS_UINT change.

Sorry.

> The LENGTH macro in intervals.h:114 has to be able to return a negative
> number.  This is probably worth reverting, until you come up with a
> proper fix.

I'd rather fix it properly.  I replaced all EMACS_UINT with EMACS_INT
in intervals.c, it couldn't be worse than int it used before.

Re: Intervals crash

[Stephen J. Turnbull]

Eli Zaretskii writes:

 > > From: Chong Yidong <cyd@stupidchicken.com>
 > > Date: Thu, 23 Sep 2010 14:23:24 -0400
 > > Cc: emacs-devel@gnu.org
 > > 
 > > I'm getting a crash due to your recent EMACS_UINT change.
 > 
 > Sorry.
 > 
 > > The LENGTH macro in intervals.h:114 has to be able to return a
 > > negative number.  This is probably worth reverting, until you
 > > come up with a proper fix.

 > I'd rather fix it properly.  I replaced all EMACS_UINT with
 > EMACS_INT in intervals.c, it couldn't be worse than int it used
 > before.

You should just do that everywhere.  EMACS_UINT is a bad idea, and
should be avoided.

First, unsigned-ness tends to propagate because of C coercion rules,
which is rarely desired; I've never seen an instance where that it
useful in Emacsen source code.

Second, unsigned integers are basically useless: ((unsigned) 0 - 1) is
not a error, it is a very big number.  This means that declaring a
non-negative variable to be unsigned buys you nothing in terms of
type-checking.  OTOH, the extra bit of precision is almost never of
interest in Emacs work.

Third, the Unix convention of using negative return values to indicate
error states mixes very badly with the first two facts: it's very easy
to inadvertantly turn a flat-out error into a big success.

The conclusion is that unsigneds (eg, size_t's) should be treated the
same way you treat legacy-encoded external text input: hazardous
material that you should convert to some sane internal type as soon as
possible, and to be produced only just before use in external APIs.
As such, there's really no need for EMACS_UINT.

FWIW, when Ben removed all unsigneds not required by external APIs
from XEmacs, not only did he fix two crashes which were associated
with an earlier misguided attempt to use size_t consistently for
non-negative variables, but two other crashes disappeared and have
never been seen again.

Re: Intervals crash

[Eli Zaretskii]

> From: "Stephen J. Turnbull" <stephen@xemacs.org>
> Cc: Chong Yidong <cyd@stupidchicken.com>,
>     emacs-devel@gnu.org
> Date: Fri, 24 Sep 2010 15:12:34 +0900
> 
>  > I'd rather fix it properly.  I replaced all EMACS_UINT with
>  > EMACS_INT in intervals.c, it couldn't be worse than int it used
>  > before.
> 
> You should just do that everywhere.  EMACS_UINT is a bad idea, and
> should be avoided.

Easier said then done, for someone such as myself, with very limited
resources.  I used EMACS_UINT because the related data structure used
it for some of its members.  I didn't have time to study the code
enough to understand whether it was really needed, but the change
which introduced it was deliberate, so I trusted that whoever did it
had good reasons.

> First, unsigned-ness tends to propagate because of C coercion rules,
> which is rarely desired

Funny that you should mention it, because I used EMACS_UINT precisely
to avoid such problems (comparison of signed and unsigned), given that
some struct members were unsigned.

> The conclusion is that unsigneds (eg, size_t's) should be treated the
> same way you treat legacy-encoded external text input: hazardous
> material that you should convert to some sane internal type as soon as
> possible, and to be produced only just before use in external APIs.
> As such, there's really no need for EMACS_UINT.

If you really need the full range of size_t or EMACS_UINT, there's
little you can do in practice.

Re: Intervals crash

[Stephen J. Turnbull]

Eli Zaretskii writes:

 > If you really need the full range of size_t or EMACS_UINT, there's
 > little you can do in practice.

Practically speaking, Emacs doesn't ever *really* need the full range
of size_t, since Lisp can't represent it in a fixnum.  It might be
nice to have a way to store them, but you can use conses of fixnums,
floats, or the occasionally-requested bignums for that.

Re: Intervals crash

[Eli Zaretskii]

> From: "Stephen J. Turnbull" <stephen@xemacs.org>
> Cc: cyd@stupidchicken.com,
>     emacs-devel@gnu.org
> Date: Fri, 24 Sep 2010 17:52:28 +0900
> 
> Eli Zaretskii writes:
> 
>  > If you really need the full range of size_t or EMACS_UINT, there's
>  > little you can do in practice.
> 
> Practically speaking, Emacs doesn't ever *really* need the full range
> of size_t, since Lisp can't represent it in a fixnum.  It might be
> nice to have a way to store them, but you can use conses of fixnums,
> floats, or the occasionally-requested bignums for that.

I didn't mean in APIs exposed to Lisp.  I meant Emacs internals.

Re: Intervals crash

[Stephen J. Turnbull]

Eli Zaretskii writes:
 > > From: "Stephen J. Turnbull" <stephen@xemacs.org>
 > > Cc: cyd@stupidchicken.com,
 > >     emacs-devel@gnu.org
 > > Date: Fri, 24 Sep 2010 17:52:28 +0900
 > > 
 > > Eli Zaretskii writes:
 > > 
 > >  > If you really need the full range of size_t or EMACS_UINT,
 > >  > there's little you can do in practice.
 > > Practically speaking, Emacs doesn't ever *really* need the full
 > > range of size_t, since Lisp can't represent it in a fixnum.  It
 > > might be nice to have a way to store them, but you can use conses
 > > of fixnums, floats, or the occasionally-requested bignums for
 > > that.
 > 
 > I didn't mean in APIs exposed to Lisp.  I meant Emacs internals.

What makes you think I don't understand that?  I will repeat what I
said earlier: *XEmacs internals don't have EMACS_UINT any more*, and
XEmacs works spectacularly better (in the sense of fewer crashes,
which I consider spectacularly bad things to have happen, YMMV), and
is easier to understand.

What's not to like about that?  Yes, I understand a lot of work needs
to be done to get there.  But (unless Emacs has some lingering
Heisencrashes that aren't understood but might be related to
infrequently called APIs that sometimes return negative numbers that
are cast to unsigned, which of course I'm sure it doesn't ;-) there's
no hurry.  Take your time; I'm just suggesting that EMACS_UINT removal
is a worthy long term goal as every such variable is an accident
waiting to happen with no compensating benefits.

That doesn't mean that we don't have unsigneds; of course we do,
because they're part of the POSIX API.  We just don't allow them to
exist in XEmacs proper any longer than it takes to test for overflow
and convert them to EMACS_INT, or to convert from EMACS_INT to
unsigned and pass to the external API.

Re: Intervals crash

[Eli Zaretskii]

> From: "Stephen J. Turnbull" <stephen@xemacs.org>
> Cc: cyd@stupidchicken.com,
>     emacs-devel@gnu.org
> Date: Sat, 25 Sep 2010 23:34:08 +0900
> 
> Eli Zaretskii writes:
>  > > From: "Stephen J. Turnbull" <stephen@xemacs.org>
>  > > Cc: cyd@stupidchicken.com,
>  > >     emacs-devel@gnu.org
>  > > Date: Fri, 24 Sep 2010 17:52:28 +0900
>  > > 
>  > > Eli Zaretskii writes:
>  > > 
>  > >  > If you really need the full range of size_t or EMACS_UINT,
>  > >  > there's little you can do in practice.
>  > > Practically speaking, Emacs doesn't ever *really* need the full
>  > > range of size_t, since Lisp can't represent it in a fixnum.  It
>  > > might be nice to have a way to store them, but you can use conses
>  > > of fixnums, floats, or the occasionally-requested bignums for
>  > > that.
>  > 
>  > I didn't mean in APIs exposed to Lisp.  I meant Emacs internals.
> 
> What makes you think I don't understand that?

And what makes you think I need a paternalistic lecture about demerits
of unsigned data types?  You already had your chance once, and I let
it go.  Why do you think I deserve it twice in a row?

Re: Intervals crash

[Stephen J. Turnbull]

Eli Zaretskii writes:

 > And what makes you think I need a paternalistic lecture about
 > demerits of unsigned data types?  You already had your chance once,
 > and I let it go.  Why do you think I deserve it twice in a row?

I don't think you deserve it.  I think Emacs deserves the benefit of
XEmacs's experience, and you were rebutting with bogus claims about
what Emacs "needs".  It would be a shame if that discourages others
from improving Emacs, that's all.

Re: Intervals crash

[Eli Zaretskii]

> From: "Stephen J. Turnbull" <stephen@xemacs.org>
> Cc: cyd@stupidchicken.com,
>     emacs-devel@gnu.org
> Date: Sun, 26 Sep 2010 03:54:55 +0900
> 
> Eli Zaretskii writes:
> 
>  > And what makes you think I need a paternalistic lecture about
>  > demerits of unsigned data types?  You already had your chance once,
>  > and I let it go.  Why do you think I deserve it twice in a row?
> 
> I don't think you deserve it.  I think Emacs deserves the benefit of
> XEmacs's experience, and you were rebutting with bogus claims about
> what Emacs "needs".  It would be a shame if that discourages others
> from improving Emacs, that's all.

How can I "rebut" theoretical arguments that are well known to every
experienced C programmer?  Who would object that in their right mind?
You didn't say anything that would be practically applicable to the
issue at hand.  You are preaching to the choir.

Did you even look at the Emacs sources, wrt to EMACS_UINT usage?  Do
you know how many times it is used?  Or are you talking out of pure
theory?

I posted a specific issue with code fragments, which gave birth to
this thread.  If you can help understand what's going on there, it
will be greatly appreciated.  Otherwise, this conversation goes
nowhere.

Re: Intervals crash

[Stephen J. Turnbull]

Eli Zaretskii writes:

 > Or are you talking out of pure theory?

Quite the opposite.  I'm speaking from pure experience: it worked very
well for XEmacs.  I happen to have a good memory for theoretical
arguments, so I passed them on.  I'm sorry that offended you.

Re: Intervals crash

[Miles Bader]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:
> Quite the opposite.  I'm speaking from pure experience: it worked very
> well for XEmacs.  I happen to have a good memory for theoretical
> arguments, so I passed them on.  I'm sorry that offended you.

"unsigned good or evil" is such a hoary old religious issue that any
unasked-for advocacy is likely to get you lumped in with the freaks,
simply out of habit on the part of the listener...

-Miles

-- 
Year, n. A period of three hundred and sixty-five disappointments.

Re: Intervals crash

[David Kastrup]

Miles Bader <miles@gnu.org> writes:

> "Stephen J. Turnbull" <stephen@xemacs.org> writes:
>> Quite the opposite.  I'm speaking from pure experience: it worked very
>> well for XEmacs.  I happen to have a good memory for theoretical
>> arguments, so I passed them on.  I'm sorry that offended you.
>
> "unsigned good or evil" is such a hoary old religious issue that any
> unasked-for advocacy is likely to get you lumped in with the freaks,
> simply out of habit on the part of the listener...

That gained bit of range turns out to be worth the pain rather rarely.
Since Emacs integers are _signed_ anyway, I have severe doubts that
EMACS_UINT will actually _ever_ give us an extended range that is not
defective to start with.  The Emacs data structure from which it is made
_is_ signed.

-- 
David Kastrup

Re: Intervals crash

[Stephen J. Turnbull]

Miles Bader writes:

 > "unsigned good or evil" is such a hoary old religious issue that
 > any unasked-for advocacy is likely to get you lumped in with the
 > freaks, simply out of habit on the part of the listener...

OK.  Nevertheless, I find it strange that the *bare fact* that
removing unsigned thoroughly has saved XEmacs users many crashes can
pass without comment (as it has, four times now).

Re: Intervals crash

[David Kastrup]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:

> Miles Bader writes:
>
>  > "unsigned good or evil" is such a hoary old religious issue that
>  > any unasked-for advocacy is likely to get you lumped in with the
>  > freaks, simply out of habit on the part of the listener...
>
> OK.  Nevertheless, I find it strange that the *bare fact* that
> removing unsigned thoroughly has saved XEmacs users many crashes can
> pass without comment (as it has, four times now).

Since few people except yourself here have experience with XEmacs, there
is not much to comment on.  That does not mean that your report went
unnoticed.  What kind of reply would you want?  "That's not relevant
since Emacs programmers are smarter than XEmacs programmers" and then
you try countering that?  A bit hard to pull off given that this thread
is about problems with EMACS_UINT in Emacs.

Personally, I was surprised that we have EMACS_UINT at all since Emacs
integers are, after all, signed, and I would not have much confidence in
applications that tried squeezing that bit out for unsigned operations.
When programming in assembly language, you have the respective
operations and status bits and conditions to pull mixed signedness
operations reasonably well.  In C, and operations have a fixed sign
interpretation for both operands and you don't get access to the
combination of carry and overflow status bits that are needed as
transitory extra precision for sorting out the ugly cases.

When you don't desperately need that extra bit of unsignedness, it is
rarely worth getting.  It's been somewhat sobering to when first
programming with the 68000 (the C compilers started out with 16bit ints)
that using _unsigned_ array index variables bloated the code and slowed
it down.  The indexed addressing modes only knew how to deal with signed
offsets.

In fact, basically any modern CPU can deal better with sign extension
(in particular in addressing modes) than with, uh, unsign extension.  So
unless you definitely need that extra bit, unsigned is not worth the
trouble.

For EMACS_UINT, we can safely say we don't need that extra bit since
Emacs integers are signed (and smaller in total size) to begin with.

Now that's not at all a comment on your practical experiences (like you
desired and will not likely get).  This is just another theoretic
rambling.

-- 
David Kastrup

Re: Intervals crash

[Jan Djärv]

2010-09-27 08:55, David Kastrup skrev:
> "Stephen J. Turnbull"<stephen@xemacs.org>  writes:

> For EMACS_UINT, we can safely say we don't need that extra bit since
> Emacs integers are signed (and smaller in total size) to begin with.
>
> Now that's not at all a comment on your practical experiences (like you
> desired and will not likely get).  This is just another theoretic
> rambling.

There are only 77 places where EMACS_UINT occurs in the Emacs sources.  Most 
of them are casts to and from pointers, and some are sizes (like in 
Lisp_Vector).  It would not be a big job to get rid of them if that is what we 
want.

	Jan D.

Re: Intervals crash

[Eli Zaretskii]

> From: Jan Djärv <jan.h.d@swipnet.se>
> Cc: emacs-devel@gnu.org
>
> There are only 77 places where EMACS_UINT occurs in the Emacs
> sources.

73, to be exact.  The rest are #define's and #ifndef's related to the
definition of EMACS_UINT itself; they don't count.  #define XUINT
should probably also be excluded, as it doesn't really "use"
EMACS_UINT.

>  Most of them are casts to and from pointers, and some are sizes
> (like in Lisp_Vector).  It would not be a big job to get rid of them
> if that is what we want.

We can get rid of almost all of them, if we believe that size_t and
EMACS_UINT are always of the same size.  I'm not sure we want to make
that assumption, though, since lisp.h does allow for external
definition of EMACS_UINT by some s/*.h or m/*.h file.

In any case, I think we cannot get rid of using an unsigned data type
in most of the 70+ places we do now, because of one or more of the
following reasons:

 . the value is a bit mask or a bit map

 . the value is a pointer that is subject to bitwise operations

 . the value is an unsigned data type forced by external hardware or
   software API

Re: Intervals crash

[David Kastrup]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Jan Djärv <jan.h.d@swipnet.se>
>> Cc: emacs-devel@gnu.org
>>
>> There are only 77 places where EMACS_UINT occurs in the Emacs
>> sources.
>
> 73, to be exact.  The rest are #define's and #ifndef's related to the
> definition of EMACS_UINT itself; they don't count.  #define XUINT
> should probably also be excluded, as it doesn't really "use"
> EMACS_UINT.
>
>>  Most of them are casts to and from pointers, and some are sizes
>> (like in Lisp_Vector).  It would not be a big job to get rid of them
>> if that is what we want.
>
> We can get rid of almost all of them, if we believe that size_t and
> EMACS_UINT are always of the same size.

Why wouldn't we be using size_t when we needed something of size size_t?

> I'm not sure we want to make that assumption, though, since lisp.h
> does allow for external definition of EMACS_UINT by some s/*.h or
> m/*.h file.
>
> In any case, I think we cannot get rid of using an unsigned data type
> in most of the 70+ places we do now, because of one or more of the
> following reasons:
>
>  . the value is a bit mask or a bit map

We have the assumption of two's complement arithmetic hardwired in a lot
of other places.  So bit operations should work on signed numbers
reasonably well.  Possible exception are right shifts when indeed the
full range of an EMACS_UINT over an EMACS_INT is being employed, but
then the number will not convert into an Elisp integer readily anyhow,
so why use EMACS_UINT at all?

>  . the value is a pointer that is subject to bitwise operations

Why would a pointer be put into an EMACS_UINT?

>  . the value is an unsigned data type forced by external hardware or
>    software API

Again, why an EMACS_UINT rather than the appropriate unsigned data type
forced by the external hardware?

If we use this as a Lisp integer, we won't be able to make use of the
full "unsigned" size.  If we don't use this as a Lisp integer, why use
EMACS_UINT in the first place?

-- 
David Kastrup

Re: Intervals crash

[Eli Zaretskii]

> From: David Kastrup <dak@gnu.org>
> Date: Mon, 27 Sep 2010 11:02:02 +0200
> 
> > We can get rid of almost all of them, if we believe that size_t and
> > EMACS_UINT are always of the same size.
> 
> Why wouldn't we be using size_t when we needed something of size size_t?

We do, as a matter of habit.  What I meant was to use size_t where we
now use EMACS_UINT, because almost all of those places have nothing to
do with Emacs integers.  We simply use EMACS_UINT as a portable
unsigned data type large enough to accommodate both size_t and a
pointer.

> > In any case, I think we cannot get rid of using an unsigned data type
> > in most of the 70+ places we do now, because of one or more of the
> > following reasons:
> >
> >  . the value is a bit mask or a bit map
> 
> We have the assumption of two's complement arithmetic hardwired in a lot
> of other places.  So bit operations should work on signed numbers
> reasonably well.  Possible exception are right shifts when indeed the
> full range of an EMACS_UINT over an EMACS_INT is being employed, but
> then the number will not convert into an Elisp integer readily anyhow,
> so why use EMACS_UINT at all?

I obviously didn't make myself clear: these are _not_ reasons to use
EMACS_UINT, these are reasons why we sometimes need an unsigned
integer data type.

>> >  . the value is a pointer that is subject to bitwise operations
> 
> Why would a pointer be put into an EMACS_UINT?

Again, not into EMACS_UINT, but rather into an unsigned data type.

The reason is, of course, that almost every Lisp_Object is a pointer
in disguise, and alloc.c was why I put this reason in the list.

> >  . the value is an unsigned data type forced by external hardware or
> >    software API
> 
> Again, why an EMACS_UINT rather than the appropriate unsigned data type
> forced by the external hardware?

See above, this isn't about EMACS_UINT.  We will need some unsigned
data type that is portable between 32- and 64-bit architectures.

> If we use this as a Lisp integer

We don't, and I didn't say we should.  I was talking about unsigned
data types that we would need even if we get rid of EMACS_UINT.

Re: Intervals crash

[Eli Zaretskii]

> From: David Kastrup <dak@gnu.org>
> Date: Mon, 27 Sep 2010 08:55:25 +0200
> 
> Personally, I was surprised that we have EMACS_UINT at all since Emacs
> integers are, after all, signed

I think it was used as a portable unsigned data type.

Re: Intervals crash

[Stephen J. Turnbull]

David Kastrup writes:

 > Personally, I was surprised that we have EMACS_UINT at all

Well, me too.  That's exactly why I spoke up.

I won't make that mistake again.  It's not worth the bruised egos.

Re: Intervals crash

[David Kastrup]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:

> David Kastrup writes:
>
>  > Personally, I was surprised that we have EMACS_UINT at all
>
> Well, me too.  That's exactly why I spoke up.
>
> I won't make that mistake again.  It's not worth the bruised egos.

"Is there anybody around capable of discussing this with me who isn't
annoying?"

Welcome to the sad and beautiful world of programmers.

-- 
David Kastrup

Re: Intervals crash

[Lars Magne Ingebrigtsen]

David Kastrup <dak@gnu.org> writes:

> "Is there anybody around capable of discussing this with me who isn't
> annoying?"
>
> Welcome to the sad and beautiful world of programmers.

Yes, but it's difficult to keep a pleasant and even tone when everybody
else are such morans!!!!1!

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Intervals crash

[Stephen J. Turnbull]

David Kastrup writes:
 > "Stephen J. Turnbull" <stephen@xemacs.org> writes:
 > 
 > > David Kastrup writes:
 > >
 > >  > Personally, I was surprised that we have EMACS_UINT at all
 > >
 > > Well, me too.  That's exactly why I spoke up.
 > >
 > > I won't make that mistake again.  It's not worth the bruised egos.
 > 
 > "Is there anybody around capable of discussing this with me who isn't
 > annoying?"

Oh, I don't kid myself.  "Is there anybody around capable of
discussing this with me that I am incapable of annoying?"  is more
like it.

Re: Intervals crash

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Thu, 23 Sep 2010 14:23:24 -0400
> Cc: emacs-devel@gnu.org
> 
> The LENGTH macro in intervals.h:114 has to be able to return a negative
> number.

Given this information, I find it difficult to reconcile the facts
with the code.  The implementation does this:

  struct interval
  {
    /* The first group of entries deal with the tree structure.  */

    EMACS_UINT total_length;      /* Length of myself and both children.  */
    EMACS_UINT position;	  /* Cache of interval's character position.  */
    ...

  /* The total size of all text represented by this interval and all its
     children in the tree.   This is zero if the interval is null. */
  #define TOTAL_LENGTH(i) ((i) == NULL_INTERVAL ? 0 : (i)->total_length)

  /* The size of text represented by this interval alone. */
  #define LENGTH(i) ((i) == NULL_INTERVAL ? 0 : (TOTAL_LENGTH ((i))          \
						 - TOTAL_LENGTH ((i)->right) \
						 - TOTAL_LENGTH ((i)->left)))

If total_length is an unsigned type, then computing negative values
from unsigned values is asking for trouble, no?  The above definitions
led me to believe that using EMACS_UINT for the result of LENGTH would
be safe, because, I reasoned, no one in their right minds would
otherwise use an unsigned type.

Also, the comment in the code that crashes:

  EMACS_INT amt = LENGTH (i);

  if (amt > 0)			/* Only used on zero-length intervals now.  */
    abort ();

lies through its teeth, doesn't it?  It implies that the result of
LENGTH is always non-negative, which again convinced me that my
understanding is correct.  But the crash says otherwise.

In addition, the commentary to delete_interval says:

  /* Delete interval I from its tree by calling `delete_node'
     and properly connecting the resultant subtree.

     I is presumed to be empty; that is, no adjustments are made
     for the length of I.  */

"I is presumed to be empty."  Is that also a lie?  Or is the code
buggy, and using an unsigned type simply exposed those bugs?

If the code is correct and the comments are wrong, I'd say we should
fix the comments, and make total_length a signed type (EMACS_INT).

Opinions and insights are most welcome.