Re: CVS commit: src/crypto/dist/openssl/apps

[David Laight <david%l8s.co.uk@localhost>]

On Wed, Apr 15, 2009 at 01:38:02AM +0700, Robert Elz wrote:
> 
> But since both you, and David, have commented on that issue, I took
> a look, and I certainly would not agree with "simply wrong".  I might
> agree with "simply useless", after all, ctyoe.h has
> 
>       int     toupper(int);
> 
> so for any call
> 
>               x = toupper(y);
> 
> 'y' is going to be converted to an int from any other type anyway,
> so changing it to be instead
> 
>               x = toupper((int)y);
> 
> is certainly useless, but cannot conceivably be "wrong" as it is changing
> nothing at all.    The original code might have been wrong, but there
> doesn't seem to be a PR about that, so if it is, I guess it doesn't bother
> anyone (I haven't analysed it, I have looked at openssl briefly in the
> past, and it made me violently ill - I have no desire to repeat the
> experience...)

The gcc warning here is 'indexing an array with char'.
The ctype functions are often implemented as #defines that index arrays,
so if the argument is 'char' it is very likely to index off the front
which leads to an unexpected result or core dump.

I suspect the gcc warning here is present precisely to detect bad use
of the ctype functiuons.

> isascii() used to be a requirement imposed upon toupper(), but no
> longer, originally (I suspect) because someone just realised that
> vast quantities of code didn't do it correctly, and the easiest fix
> was just to alter the implementation, rather than attempt to
> enforce that rule - so it was deleted years ago.

Actually islower() used to be a requirement, old SYSV systems used the
same array for toupper() and tolower().

        David

-- 
David Laight: david%l8s.co.uk@localhost