Re: pango patch -- fix some warnings, at least one bug

[Owen Taylor <otaylor redhat com>]

Darin Adler <darin bentspoon com> writes:

> On Thursday, July 12, 2001, at 08:49  AM, Owen Taylor wrote:
> 
> >> Index: modules/arabic/arabic-x.c
> >> ===================================================================
> >> RCS file: /cvs/gnome/pango/modules/arabic/arabic-x.c,v
> >> retrieving revision 1.21
> >> diff -p -u -r1.21 arabic-x.c
> >> --- modules/arabic/arabic-x.c	2001/06/25 15:09:39	1.21
> >> +++ modules/arabic/arabic-x.c	2001/07/11 23:39:32
> >> @@ -178,7 +178,7 @@ arabic_engine_shape (PangoFont        *f
> >>                        PangoGlyphString *glyphs)
> >>   {
> >>       PangoXSubfont   subfont;
> >> -    long            n_chars;
> >> +    int             n_chars;
> >>       int             i;
> >>       ArabicFontInfo *fs;
> >>       const char     *p;
> >
> > I think it would be marginally better to change the argument to
> > arabic_reshape() instead.
> 
> I looked into this, and it's not simple to do. The arabic_reshape
> function passes the int* to functions named shape and doublelig. These
> do a lot of manipulation on the length. Changing the parameter to
> arabic_reshape is basically fixing all the code to handle the length
> type correctly. Changing the local variable to int is going with the
> flow of what's there.
> 
> Given that, are you sure that you want me to tackle the more-ambitious
> change? It's probably only about 10 minutes work, but I'm worried
> about doing it with no way to test if my changes broke anything.

No real strong feeling either way - I guess just stick with the simple
change.

The only real advantage of using the long is that a someone trying to
detect 64-bit "errors" won't notice an implicit cast from a long to an
int.

But its completely and utterly improbable that n_chars here could
exceed the range of an int. (A 2 billion character paragraph?)

Regards,
                                        Owen