Bug 28432

Summary: String::createCFString() returns a non-retained empty string
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: PlatformAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Minor    
Priority: P4    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
proposed patch darin: review+

Alexey Proskuryakov
Reported 2009-08-18 10:46:58 PDT
If String is null, createCFString() returns CFSTR(""). According to CFString.h, 'CFSTR(), not being a "Copy" or "Create" function, does not return a new reference for you. So, you should not release the return value.' I am not aware of any practical issues caused by this, but we should probably comply.
Attachments
proposed patch (1.18 KB, patch)
2009-08-18 10:49 PDT, Alexey Proskuryakov 		darin: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2009-08-18 10:49:02 PDT
Created attachment 35051 [details] proposed patch
Mark Rowe (bdash)
Comment 2 2009-08-18 11:01:46 PDT
Comment on attachment 35051 [details] proposed patch > + * platform/text/cf/StringCF.cpp: (WebCore::String::createCFString): Rather than retain > + CFSTR result, we can just return a new string, this doesn't seem to be a hot code path Even though it's not a hot code path it seems like we should avoid the extra memory allocation that CFStringCreateWithCharacters does compared to CFSTR.
Darin Adler
Comment 3 2009-08-18 11:01:55 PDT
Comment on attachment 35051 [details] proposed patch If either solution is fine speed-wise, then why not call CFRetain to save memory in case a lot of these strings are kept around?
Alexey Proskuryakov
Comment 4 2009-08-18 13:02:40 PDT
Committed revision 47449. > If either solution is fine speed-wise, then why not call CFRetain to save > memory in case a lot of these strings are kept around? OK, changed. /me doesn't like to share objects across threads.
Note You need to log in before you can comment on or make changes to this bug.