242760 - code cleaning: rechecking checked rc's, dropping a temporary comptr, a comment change

Status: RESOLVED FIXED

(Core Graveyard :: Web Services, defect)

Description

[timeless]

I recently ran into a spot where an unchecked rv resulted in a crash in soap code.

Today I was browsing (I'm hunting for a specific item, but finding what I want
is harder than finding stuff I don't like), and noticed that there were places
which did this:

if (NS_FAILED(rc))
  return rc;
p->SetAsAString(aExternalURI);
if (NS_FAILED(rc))
  return rc;

It's pretty, and as caillon notes, hopefully the duplicate check is optimized
away. problem is, the check is intended and wants to do work, but it's missing
the step to store the result.

I used:
R:\mozilla\extensions\webservices\soap\src>grep -B 4 "if (NS_FAILED(rc))" *.cpp
to hunt for these items. The alloc check is of course a pet peeve.

Comment 1

[timeless]

Attachment: store rc's so ifs can check, drop temporary comptr, change comment, check alloc

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 147787 [details] [diff] [review]
store rc's so ifs can check, drop temporary comptr, change comment, check alloc

-      element->AppendChild(text, getter_AddRefs(ignore));
+      rc = element->AppendChild(text, getter_AddRefs(ignore));
       if (NS_FAILED(rc)) 
       return rc;

Wanna fix the indentation there while you're in the neighborhood?

Very nice catches!

r+sr=jst

Comment 3

[timeless]

mozilla/extensions/webservices/soap/src/nsDefaultSOAPEncoder.cpp 	1.83