On Sat, 28 Jan 2006 01:21:22 -0600 James Hawkins truiken@gmail.com wrote:
On 1/28/06, Steven Edwards winehacker@gmail.com wrote:
Hello, Ok here are some proposed ground rules for the audit. Mostly thanks to Art and Alex. We are still open for debate on this
...
- A function is deemed to have been implemented in a non-clean manner if
- functions for which there is NO DOCUMENTATION
I think this condition isn't always evidence of a non-clean implementation of a function. A developer might have written tests for a function and not committed them back to ReactOS. This could be rewritten as:
"If a function has NO DOCUMENTATION and no test cases exist either in ReactOS or elsewhere, then test cases must be submitted to ReactOS."
Actually, that appears 4 lines later in the original.
- functions with excessive gotos
This case is similar to the documentation case in that it's not direct evidence of a non-clean implementation. I frequently use gotos for releasing many resources in error cases as a way of factoring code. A better wording would be:
"Functions with excessive gotos should be marked for further inspection."
-- James Hawkins
Nobody's saying 'oh noes ... we must toss all code with gotos'. These are metrics for functions that we must consider as needing more than cursory examination. I was trying to set some boundaries for clearly in and clearly out that can be measured in a somewhat objective way.
To me, using more than two or three goto targets per function is excessive. Goto targets for cleanup and generic error handling are pretty common in C code, but I believe that code which uses more than a handful of gotos should at least be examined in detail.