Date: Tue, 14 Apr 2009 19:59:46 +0200 From: Joerg Sonnenberger <jo...@britannica.bec.de> Message-ID: <20090414175946.ga12...@britannica.bec.de>
| toupper like the rest of ctype.h is defined for EOF and for all values | of a unsigned char. Yes. Which is why its arg cannot be u_char as you asserted in the previous message. Which incidentally is the only point upon which I was commenting in my message. I'm not about to get into discussions about the adequacy of the ctype macros/functions (and EOF) on mythical architectures (whether or not any ever existed in the dim past) where sizeof(u_char) == sizeof(int), such things are just a waste of anyone with anything real to do's time and energy - leave that to the language idiots who keep butchering C so it can work on those non-existent systems even if anyone was really silly enough to care to make it. I didn't (mean to) comment on the "This cast is simply wrong" part of your message, as I hadn't loooked at the code - I probably should have edited that but out of my quote (but it is much easier to just delete whole lines...) 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...) I'm also not about to enter into any flame wars about the sanity of many of gcc's warnings, or their usefulness, or even whether it is better to ignore the things, disable them, or pander to them - that kind of discussion also isn't worth anyone's time. | Nevertheless, casting to int just to shut up a | warning from GCC is almost always the wrong approach here, unless it has | been explicitly checked before that isascii() is true and I can't find | immediate evidence for that. 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. More recently of course, this also permits toupper() to work on code sets that are not ASCII, like Greek (in theory anyway, not on NetBSD, or not yet anyway), which ought to be a good thing, right? (That is of course, just the 1 byte codesets, which still makes it defective, but not as bad as being ASCII only.) kre