On 26/09/2007, chromatic <[EMAIL PROTECTED]> wrote: > On Wednesday 26 September 2007 01:16:54 [EMAIL PROTECTED] wrote: > > > Author: paultcochrane > > Date: Wed Sep 26 01:16:53 2007 > > New Revision: 21597 > > > > Modified: > > trunk/languages/m4/src/eval.c > > > > Log: > > [m4] Cast isxxx() function arguments explicitly to unsigned char as per > > coding standards. > > I don't understand this patch. *eval_text is explicitly defined in that file > as an unsigned char *. Why cast it? At best, a smart compiler will see that > it's unnecessary and ignore it. At worse, it'll confuse readers who think > that the code may be doing the wrong thing. (An explicit cast is probably > *dangerous* if the type of the argument is wrong, as it'll likely have > signedness problems and we won't even get a compiler warning.) > > I've read the paragraph talking about this in PDD 07 and I don't see anything > that requires an explicit cast when the type is appropriate. I interpret the > rule as saying "If you pass in a signed number, you're doing it wrong, so > don't ever do that; it's a source of bugs."
I've reverted the patch in r21617. I've also removed the test from the main test harness as it can't (presently) handle situations in which the code is correctly declared as unsigned char and so doesn't need to be explicitly cast in the function call. Many thanks for the code review! Paul
