Closed Bug 203724 Opened 22 years ago Closed 20 years ago

NS_PRECONDITION: Truth in advertising (2)

Categories

(Core :: XPCOM, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: bfowler, Assigned: dougt)

Details

User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.4b) Gecko/20030418 Chimera/0.7+ Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.4b) Gecko/20030418 Chimera/0.7+ Precondition 'statements' are general written by experienced engineers, or at least by those who fully understand the details of the code. They should be taken by the rest of us at full face value. Execution should never continue past an unmet pre-condition. One way of achieving this would be for code that has pre-conditions to return a known rv for 'Pre-condition not met', and (like an exception) its callers should either handle the situation or pass it upwards. As an example, see this code from nsPresShell.cpp: NS_IMETHODIMP PresShell::CreateRenderingContext(nsIFrame *aFrame, nsIRenderingContext** aResult) { NS_PRECONDITION(nsnull != aResult, "null ptr"); if (nsnull == aResult) { return NS_ERROR_NULL_POINTER; } // .... It checks the precondition, and then has to make the same check again to do somthing about it. IMHO, the preprocessor should be responsible for the second part, once it has been decided that a PRECONDITION statement is appropriate. It follows that PRECONDITION statements need to generate appropriate code in non-debug builds. Reproducible: Always Steps to Reproduce: 1. Mess up a function so that a precondition is not met 2. Trace the program flow in a debugger 3. Actual Results: Note that in many (arguably highly inappropriate) cases such as inside the implemementation of nsCOMPtr, program execution continues past the precondition and into the main body which should have been protected. Expected Results: Code protected by a PRECONDITION should not be run (ever) if the precondition is not met. Truth in advertising.
-> mozilla
Assignee: saari → dougt
Component: General → XPCOM
Product: Camino → Browser
QA Contact: winnie → scc
Version: unspecified → Trunk
Nominalism aside, is there anything to do here other than change NS_PRECONDITION to NS_ENSURE_TRUE, and remove any redundant return-if-false code that follows any such macro calls? I think this bug is WONTFIX. /be
I agree that this should be wontfix, but they should probably be fatal in debug builds. NS_PRECONDITION has always been used for something that is a precondition that should not be checked at runtime in release builds; NS_ENSURE_TRUE has been used for those that should be checked at runtime.
Status: UNCONFIRMED → RESOLVED
Closed: 20 years ago
Resolution: --- → WONTFIX
(Bug 279923 covers making assertions fatal in debug builds.)
(In reply to comment #3) > ... NS_PRECONDITION has always been used for something that is a > precondition that should not be checked at runtime in release builds; > NS_ENSURE_TRUE has been used for those that should be checked at runtime. Now I appreciate that a careful reading of what I have written reveals that I am largely proving a truism that a "little knowledge is a dangerous thing", here. Yes, Preconditions should be part of unit testing and should be turned off in debugged programs. http://www.devchannel.org/devtoolschannel/03/08/18/212243.shtml?tid=39 My original request is meaningless, and should indeed be closed as INVALID in some form.
You need to log in before you can comment on or make changes to this bug.