Leak of array if rv != NS_OK in BuildSearchElements()

RESOLVED FIXED in mozilla1.9.1a1

Status

MailNews Core
Address Book
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Håkan Waara, Assigned: Håkan Waara)

Tracking

({mlk})

Trunk
mozilla1.9.1a1
All
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.44 KB, patch
standard8
: review+
Details | Diff | Splinter Review
(Assignee)

Description

10 years ago
BuildSearchElements() in nsAbOSXDirectory.mm will leak an NSArray and all its items if it's returning early with NS_ENSURE_SUCCESS(rv, rv).

Fix coming up.
(Assignee)

Comment 1

10 years ago
Created attachment 325130 [details] [diff] [review]
Fix v1

Fix to make sure we always escape from the loop (instead of returning early) on error.
Attachment #325130 - Flags: review?(bugzilla)
Comment on attachment 325130 [details] [diff] [review]
Fix v1

     if (!aCanHandle) {
-      return NS_OK;
+      rv = NS_OK;
+      break;
     }

The way I read this, we'll be processing the if (array) statement below in the !aCanHandle case, whereas we wouldn't have before.

I don't think that matters, but could you see if you agree, and if so, please could you add some comments onto BuildSearchElements stating that if aCanHandle is false, the result should be ignored?
Attachment #325130 - Flags: review?(bugzilla) → review-
(Assignee)

Comment 3

10 years ago
Created attachment 326695 [details] [diff] [review]
Fix v2
Attachment #325130 - Attachment is obsolete: true
Attachment #326695 - Flags: review?(bugzilla)
Attachment #326695 - Flags: review?(bugzilla) → review+
(Assignee)

Comment 4

10 years ago
Checked in.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Updated

10 years ago
Hardware: PC → All
Target Milestone: --- → mozilla1.9.1a1
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.