283238 - Audit mozilla/mailnews/base/src/nsMsgDBView.cpp

Status: RESOLVED WORKSFORME

(MailNews Core :: Database, defect)

Description

[timeless]

audit issues:
unchecked allocs => don't crash (or leak)
leaking on oom => don't leak :)
else after return/return after else => drop the else and fix the indentation
ns_assertion(0,...) => use ns_error

Comment 1

[timeless]

hrm, to be clear, for return after else, you need to negate the if condition,
promote the return and then unindent the main block.

if (foo) {
  statement;
  statement;
} else
  return blah;

=>

if (!foo)
  return blah;
statement;
statement;

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Gandalf, still working on this?

(file in question is at: http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgDBView.cpp )

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

didn't touch it in 4 years, so feel free to takeover. :)

Comment 4

[Wayne Mery (:wsmwk)]

perhaps also bug 456332 and bug 460036?  If so, this is likely also causing topcrashes

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #0)
> ns_assertion(0,...) => use ns_error

More global issue, moved to bug 489505.

Comment 6

[Wayne Mery (:wsmwk)]

(In reply to comment #0)
> audit issues:
> 1. unchecked allocs => don't crash (or leak)
> 2. leaking on oom => don't leak :)
> 3. else after return/return after else => drop the else and fix the indentation
> 4. ns_assertion(0,...) => use ns_error

Is there anything here left to do? 
I skimmed the source and it looks like 1,3,4 are done. 
I this also done? 
  > leaking on oom => don't leak :)

Comment 7

[Joshua Cranmer [:jcranmer]]

(In reply to comment #6)
> I this also done? 
>   > leaking on oom => don't leak :)

The mozilla codebase has been moving to an abort-on-oom for failure, so out-of-memory should just cause the program to crash instead of merely leaking...

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Resolve WFM then. :)