Bug 28432 - String::createCFString() returns a non-retained empty string
Summary: String::createCFString() returns a non-retained empty string
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P4 Minor
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-18 10:46 PDT by Alexey Proskuryakov
Modified: 2009-08-18 13:02 PDT (History)
0 users

See Also:


Attachments
proposed patch (1.18 KB, patch)
2009-08-18 10:49 PDT, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 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.
Comment 1 Alexey Proskuryakov 2009-08-18 10:49:02 PDT
Created attachment 35051 [details]
proposed patch
Comment 2 Mark Rowe (bdash) 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.
Comment 3 Darin Adler 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?
Comment 4 Alexey Proskuryakov 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.