464159 - Quickstubs leaks IIDs

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Robert Sayre]

LookupInterfaceOrAncestor needs to free the variable "piid"

Comment 1

[Robert Sayre]

Attachment: obvious patch

valgrind says this fixes most of the leaks, but not all. That's confusing, because I'm not sure how it could still leak.

Comment 2

[Jason Orendorff [:jorendorff]]

I think valgrind is right.  GetInterfaceIID is called within the loop, so it could be called several times.  So the Free call needs to be inside the loop as well, right?

             if(NS_FAILED(info->GetParent(getter_AddRefs(parent))) ||
                !parent ||
                NS_FAILED(parent->GetInterfaceIID(&piid)))
             {
                 break;
             }
             p = LookupEntry(tableSize, table, *piid);
+            nsMemory::Free(piid);
             if(p)
                 break;

No need to initialize piid to null; it could be moved inside the loop though.

Also feel free to rename "p" to "entry" or something.  I don't know what I was thinking.  :-\

Comment 3

[Benjamin Smedberg]

Or better yet, use GetIIDShared and avoid the allocation/free pair altogether.

Comment 4

[Robert Sayre]

(In reply to comment #2)
> I think valgrind is right.  GetInterfaceIID is called within the loop, so it
> could be called several times.  So the Free call needs to be inside the loop as
> well, right?

Oh, duh, yeah. I'll get something going incorporating Benjamin's suggestion.

Comment 5

[Robert Sayre]

Attachment: use GetIIDShared

Comment 6

[Jason Orendorff [:jorendorff]]

Comment on attachment 348237 [details] [diff] [review]
use GetIIDShared

Looks good.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 348237 [details] [diff] [review]
use GetIIDShared

a191=beltzner

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/c78fb7914c0c

Comment 9

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d5a2ce166acf