Summary: | [EFL][GTK] Remove use of String::format() in versionForUAString() | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||
Component: | WebKit EFL | Assignee: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, lucas.de.marchi | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 30342, 151340 | ||||||||||||
Attachments: |
|
Description
Gyuyoung Kim
2015-11-13 00:51:20 PST
Created attachment 265473 [details]
Patch
Comment on attachment 265473 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265473&action=review > Source/WebCore/platform/efl/UserAgentEfl.cpp:64 > static const String& versionForUAString() > { > - static NeverDestroyed<String> version(String::format("%i.%i", WEBKIT_MAJOR_VERSION, WEBKIT_MINOR_VERSION)); > +#define MAKE_VERSION(major, minor) #major "." #minor > + const String& version = MAKE_VERSION(WEBKIT_MAJOR_VERSION, WEBKIT_MINOR_VERSION); > return version; > +#undef MAKE_VERSION > } This is wrong. It will return a reference to an already-destroyed string object. Comment on attachment 265473 [details]
Patch
Here’s how to do it:
static const char* versionForUAString()
{
#define MAKE_VERSION(major, minor) #major "." #minor
return MAKE_VERSION(USER_AGENT_GTK_MAJOR_VERSION, USER_AGENT_GTK_MINOR_VERSION);
#undef MAKE_VERSION
}
In the EFL case we also need to change the type from const String& to const char*.
Created attachment 265541 [details]
Patch
(In reply to comment #3) > Comment on attachment 265473 [details] > Patch > > Here’s how to do it: > > static const char* versionForUAString() > { > #define MAKE_VERSION(major, minor) #major "." #minor > return MAKE_VERSION(USER_AGENT_GTK_MAJOR_VERSION, > USER_AGENT_GTK_MINOR_VERSION); > #undef MAKE_VERSION > } > > In the EFL case we also need to change the type from const String& to const > char*. I see. I update the patch as well as platformVersionForUAString() is modified. Comment on attachment 265541 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265541&action=review > Source/WebCore/platform/gtk/UserAgentGtk.cpp:106 > + return MAKE_VERSION(name.sysname, name.machine); This won’t work. I believe this will yield the string: "name.sysnamename.machine" This change should not be made. Testing would have shown this is incorrect. > Source/WebCore/platform/gtk/UserAgentGtk.cpp:109 > + return MAKE_VERSION(cpuDescriptionForUAString(), "Mac OS X"); This won’t work. I believe this will yield the string: "cpuDescriptionForUAString()Mac OS X" This change should not be made. Testing would have shown this is incorrect. Created attachment 265548 [details]
Patch
Comment on attachment 265548 [details]
Patch
standardUserAgent in UserAgentEfl.cpp can still be improved so it doesn’t convert the const char* returned by versionForUAString to a String for the local variable version every time it’s called
Created attachment 265549 [details]
Patch
(In reply to comment #8) > Comment on attachment 265548 [details] > Patch > > standardUserAgent in UserAgentEfl.cpp can still be improved so it doesn’t > convert the const char* returned by versionForUAString to a String for the > local variable version every time it’s called const char* is converted to a String using ASCIILiteral in new patch. Comment on attachment 265549 [details] Patch Clearing flags on attachment: 265549 Committed r192459: <http://trac.webkit.org/changeset/192459> All reviewed patches have been landed. Closing bug. |