Re: Fix, NSLayoutManager

[Alexander Malmberg]

Fred Kiefer wrote:
> Georg Fleischmann wrote:
> > Hi,
> >
> > here is a little patch for the NSLayoutManager fixing a problem with
> > layout_char. layout_char is unsigned but may become "negative", thus 
> > flipping
> > over to huge positive. The huge positive value then is not sattisfying the 
> > '<'
> > comparison.
[snip]
> > --- 1800,1806 ----
> >     if (layout_char > r.location)
> >       {
> >         layout_char += lengthChange;
> > !       if (layout_char < r.location || layout_char > r.location + r.length)
> >           layout_char = r.location;
> >       }
> 
> I think you did spot a real problem here, but your solution just doesn't
> look right. An unsigned number never should be allowed to wrap around.
> What about a check like this:

This has the same problem; there's an implicit wrap around...

>      if (layout_char > r.location)
>        {
>         if (layout_char + lengthChange < r.location)
... here:     ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^

However, in the original patch, the wrap-around is just ugly; it isn't
really a problem since the wrap-around behavior of unsigned ints is
defined by the c standard.

I've committed a slightly different patch that's both pretty and safe,
and makes the intent of the code more clear (I hope): we don't care what
layout_char is if it was inside the range of changed characters, we just
want to notice that it was so we pick the right branch on the if further
down:

  if (layout_char > r.location)
    {
      if (layout_char >= r.location + r.length)
        layout_char += lengthChange;
      else
        layout_char = r.location;
    }

(The current behavior seems rather questionable when I look at it now,
but I don't think there was any other real problem with it. layout_char
would get a weird value in some cases, but it'd be recalculated further
down in all interesting cases. Either way, this will be safer.)

Thanks for finding this! :)

- Alexander Malmberg