Proposal for change in standard anti-crash C++ checks

Ben Bucksch 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 
> standards.
>
> 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:
>
>

Do both:
nsCOMPtr<nsIMsgIncomingServer> server;
nsresult rv = GetServer(getter_AddRefs(server));
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_STATE(server);
return server->GetMsgStore(aStore);

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 mailing list