447994 - <nsImapService.cpp> has unused assignments

Status: NEW

(MailNews Core :: Networking: IMAP, defect)

Description

[Serge Gautherie (:sgautherie)]

<http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapService.cpp>
{{
635     rv = url->SetSpec(urlSpec);
636     
637     rv = aImapUrl->SetImapAction(actionToUse /* nsIImapUrl::nsImapMsgFetch */);
}}

Remove |rv = | at line 635, or check its value.

***

Same with
{{
1018   rv = url->SetSpec(urlSpec);
...
1063   rv = aImapUrl->SetImapAction(aImapAction);
}}

and
{{
3027     rv = imapUrl->SetImapAction(nsIImapUrl::nsImapUserDefinedFetchAttribute);
...
3032     rv = SetImapUrlSink(anImapFolder, imapUrl);
}}

... Basically, check the whole file.

Comment 1

[Sander Goudswaard]

Do the following crashes relate to this bug?
http://crash-stats.mozilla.com/report/index/e6c08868-7ccd-11dd-8e3f-001a4bd43ef6?p=1

and

http://crash-stats.mozilla.com/report/index/bb9a6a94-7ccc-11dd-b24b-001cc4e2bf68?p=1

Comment 2

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)
> Do the following crashes relate to this bug?

"Thunderbird 3.0b1pre Crash Report [@ nsImapService::CreateStartOfImapUrl(nsACString_internal const&, nsIImapUrl**, nsIMsgFolder*, nsIUrlListener*, nsACString_internal&, unsigned short&) ]"

Doubtful, unless it would be prevented by checking these return values.
For the time being, you should rather search/file a new bug.

Comment 3

[:aceman]

The setSpec cases should be covered in bug 186724.

There are many spots where rv doesn't seem to be checked after assignment.

David Bienvenu, are you still working on this? Is this just a matter of adding the checks everywhere (with a bit of sense)?

Comment 4

[David :Bienvenu]

(In reply to :aceman from comment #3)
> The setSpec cases should be covered in bug 186724.
> 
> There are many spots where rv doesn't seem to be checked after assignment.
> 
> David Bienvenu, are you still working on this? Is this just a matter of
> adding the checks everywhere (with a bit of sense)?
and sometimes removing the rv assignment, but yes, this is simple, and I'm not working on it.

Comment 5

[:aceman]

And how can I decide where to remove the assignment?