Bug 6191 - RefPtr/PassRefPtr have a leak issue, operator== issues
Summary: RefPtr/PassRefPtr have a leak issue, operator== issues
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Darin Adler
URL:
Keywords:
: 6165 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-12-21 21:56 PST by Darin Adler
Modified: 2005-12-22 08:46 PST (History)
1 user (show)

See Also:


Attachments
patch that fixes the issues described in the bug (14.18 KB, patch)
2005-12-21 22:02 PST, Darin Adler
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2005-12-21 21:56:06 PST
I have a patch that fixes these things:

    1) adds a constructor that lets you mix PassRefPtr objects of compatible types
    2) optimizes PassRefPtr::adopt so it doesn't first set m_ptr to 0
    3) allows assignment of PassRefPtr objects from a RefPtr of any compatible type and vice versa
    4) fixes a leak in the type conversion operator that converts a PassRefPtr to one of another compatible 
type
    5) allows mixing compatible types in == and != for PassRefPtr and RefPtr
    6) removes extraneous const in == and != operator implementations (which was probably harmless)
Comment 1 Darin Adler 2005-12-21 22:00:46 PST
Marking P1 because of the potential leak.
Comment 2 Darin Adler 2005-12-21 22:02:02 PST
Created attachment 5212 [details]
patch that fixes the issues described in the bug
Comment 3 Darin Adler 2005-12-21 22:33:59 PST
*** Bug 6165 has been marked as a duplicate of this bug. ***
Comment 4 Maciej Stachowiak 2005-12-22 02:34:16 PST
Why this part: "Remove non-template constructor that takes RefPtr [from PassRefPtr]."

Other than that, all changes look great to me.
Comment 5 Maciej Stachowiak 2005-12-22 02:57:49 PST
r=me assuming you add a good explanation of why the PassRefPtr constructor that takes RefPtr was 
removed.
Comment 6 Darin Adler 2005-12-22 07:31:40 PST
Here's the rationale: There's still a template constructor that takes a RefPtr in PassRefPtr that should cover 
any RefPtr. I believe there's no need for a non-template constructor specifically for a RefPtr to the same 
class as the PassRefPtr.
Comment 7 Darin Adler 2005-12-22 07:34:41 PST
I'm not entirely clear in what situations it's helpful to have both a constructor and constructor template 
that overlap. I believe in the case of constructing a PassRefPtr from a RefPtr that the constructor template 
suffices by itself and we don't also need the non-template constructor.