Proposal for change in standard anti-crash C++ checks
ben.bucksch at beonex.com
Thu Nov 21 21:40:05 UTC 2013
Kent James wrote, On 06.11.2013 01:18:
> I'd like to have a small discussion about standard ways of checking
> for errors in C++ code as anti-crash prevention, and propose two code
> 1) A common pattern in mailnews code is the following:
> nsCOMPtr<nsIMsgIncomingServer> server;
> nsresult rv = GetServer(getter_AddRefs(server));
> NS_ENSURE_SUCCESS(rv, rv);
> return server->GetMsgStore(aStore);
> I would like to propose that, instead, the pattern be the following
> when all we are doing is preventing crashes, but have every
> expectation that the getter should succeed:
nsresult rv = GetServer(getter_AddRefs(server));
This way, you pass up the nsresult, but you also check that it's not null.
To answer aceman: if the code wrongly expects a non-null result, at
least it wouldn't crash anymore, just error, so it's an improvement.
Code that can return null must document that in the callee, and should
document it in the caller, too. This way, it's clear that the code
either expects a non-null (NS_ENSURE_STATE()), or knows that it may be
null (with the comment), or that it's not fixed yet (neither of both).
> sometimes we may have code that intentionally returns a null value, as
> is being proposed in bug 739908 for spamSettings. Nobody really knows
> which code follows which standard
That's the real problem here.
More information about the tb-planning