Bug 160879 - Add an address-of operator to RetainPtr
Summary: Add an address-of operator to RetainPtr
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-15 16:34 PDT by Anders Carlsson
Modified: 2016-08-16 12:31 PDT (History)
5 users (show)

See Also:


Attachments
Patch (75.92 KB, patch)
2016-08-15 16:37 PDT, Anders Carlsson
no flags Details | Formatted Diff | Diff
Patch (77.75 KB, patch)
2016-08-16 09:43 PDT, Anders Carlsson
no flags Details | Formatted Diff | Diff
Patch (77.69 KB, patch)
2016-08-16 10:21 PDT, Anders Carlsson
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2016-08-15 16:34:03 PDT
Add an address-of operator to RetainPtr
Comment 1 Anders Carlsson 2016-08-15 16:37:30 PDT
Created attachment 286117 [details]
Patch
Comment 2 Alex Christensen 2016-08-15 20:50:19 PDT
Comment on attachment 286117 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=286117&action=review

So if I have a RetainPtr<something> x; and I ever do &x, then x is now null?  That's strange.

> Source/WTF/wtf/RetainPtr.h:95
> +            CFRelease(ptr);

This isn't tested.
Comment 3 Csaba Osztrogonác 2016-08-15 23:06:30 PDT
Please remove the unrelated xcodeproj file changes.
Comment 4 Darin Adler 2016-08-15 23:28:10 PDT
Comment on attachment 286117 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=286117&action=review

> Source/WTF/wtf/HashIterators.h:154
> +        MappedType* get() const { return std::addressof(m_impl.get()->value); }

Should change this file to use #pragma once.

> Source/WTF/wtf/HashTable.h:851
> +            memset(std::addressof(bucket), 0, sizeof(bucket));

Should change this file to use #pragma once.

> Source/WTF/wtf/RetainPtr.h:92
> +    PtrType* operator&()

I think this needs a comment explaining what it does.

>> Source/WTF/wtf/RetainPtr.h:95
>> +            CFRelease(ptr);
> 
> This isn't tested.

Not sure what you mean exactly.

> Source/WTF/wtf/RetainPtr.h:97
> +        return &m_ptr;

Compilation failure on Windows in CookieJarCFNet.cpp:

WTF/wtf/RetainPtr.h(97): error C2440: 'return': cannot convert from 'WTF::RetainPtr<CFStringRef>::StorageType *' to 'const __CFString **' (compiling source file WebCore\platform\network\cf\CookieJarCFNet.cpp)
WTF/wtf/RetainPtr.h(97): note: Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast (compiling source file WebCore\platform\network\cf\CookieJarCFNet.cpp)
WTF/wtf/RetainPtr.h(93): note: while compiling class template member function 'const __CFString **WTF::RetainPtr<CFStringRef>::operator &(void)' (compiling source file WebCore\platform\network\cf\CookieJarCFNet.cpp)
WebCore\platform\network\cf\CookieJarCFNet.cpp(145): note: see reference to function template instantiation 'const __CFString **WTF::RetainPtr<CFStringRef>::operator &(void)' being compiled
WebCore\platform\network\cf\CookieJarCFNet.cpp(58): note: see reference to class template instantiation 'WTF::RetainPtr<CFStringRef>' being compiled
Comment 5 Anders Carlsson 2016-08-16 09:43:15 PDT
Created attachment 286181 [details]
Patch
Comment 6 Anders Carlsson 2016-08-16 10:21:34 PDT
Created attachment 286182 [details]
Patch
Comment 7 Anders Carlsson 2016-08-16 10:28:30 PDT
(In reply to comment #2)
> Comment on attachment 286117 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=286117&action=review
> 
> So if I have a RetainPtr<something> x; and I ever do &x, then x is now null?
> That's strange.

I did it to avoid memory leaks, but I do agree that it's weird and so now I'm just asserting that x is null.
Comment 8 Anders Carlsson 2016-08-16 12:31:12 PDT
Committed r204519: <http://trac.webkit.org/changeset/204519>