Rick Altherr wrote: > I personally find the excerpted line confusing and clunky. I _do_ > know what it does, but only because I've written the exact same code > to handle the error cases returned by stroul. Without seeing the rest > of the code, here's what I don't like about it: > - okay is a poor choice of variable name. Its name declares that it > is "okay", but it really is telling you _if_ something is okay. > is_okay is much clearer. > - There are really two separate tests being done sequentially. The > first is to see if the whole string was used to generate the number. > The second is to see if the parsed number was larger than an unsigned > log. That isn't immediately clear even though operator precedence and > associativity means it evaluates that way. > - The use of 'okay' seems potentially unnecessary. Even if the test > is being used to determine multiple control flow paths, the code can > usually be changed to only need a single if. The variable declaration > doesn't really add anything to the clarity.
I think leaving out the parentheses is perfectly OK in this case, but the conditions might be rearranged (invert & exit, split into two separate tests) to make the code easier to understand. > - If you are testing that a variable is non-NULL, write that. > Similarly for a test for NULL. Taking the short-cut just makes it > less clear what you are doing and why. Besides, it isn't necessarily > guaranteed that NULL will always be 0. ;) NULL is for pointers, nothing else. If you want to test if something is zero, write "0". IMHO, even in pointer context, "0" should be used - "NULL" provides no extra function except hiding what happens, and using if (pointer == 0) instead of if (pointer) is no improvement at all. And yes, it *is* guaranteed that NULL is 0, see: http://c-faq.com/null/index.html cu Michael _______________________________________________ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
