Bug 183545

Summary: PerProcess<> should be safe by default
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: bmallocAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, ggaren, joepeck, lforschler, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
maybe this will work
none
the patch
ysuzuki: review+
patch for landing none

Filip Pizlo
Reported 2018-03-10 13:19:47 PST
Patch forthcoming.
Attachments
maybe this will work (14.08 KB, patch)
2018-03-10 13:20 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (16.37 KB, patch)
2018-03-10 15:24 PST, Filip Pizlo 		ysuzuki: review+
DetailsFormatted DiffDiff
patch for landing (16.92 KB, patch)
2018-03-11 10:18 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2018-03-10 13:20:25 PST
Created attachment 335519 [details] maybe this will work
Filip Pizlo
Comment 2 2018-03-10 15:24:03 PST
Created attachment 335520 [details] the patch
EWS Watchlist
Comment 3 2018-03-10 15:27:15 PST
Attachment 335520 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/PerProcess.h:64: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/PerProcess.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 2 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 4 2018-03-11 01:34:22 PST
Comment on attachment 335520 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=335520&action=review r=me with a micro nit. > Source/bmalloc/bmalloc/PerProcess.cpp:49 > +static unsigned stringHash(const char* string) > +{ > + unsigned result = 5381; > + while (char c = *string++) > + result = result * 33 + c; > + return result; > +} Nits: you can make this `constexpr` function, move it to a header, and calculate hash value at compile time.
Filip Pizlo
Comment 5 2018-03-11 10:15:15 PDT
(In reply to Yusuke Suzuki from comment #4) > Comment on attachment 335520 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=335520&action=review > > r=me with a micro nit. > > > Source/bmalloc/bmalloc/PerProcess.cpp:49 > > +static unsigned stringHash(const char* string) > > +{ > > + unsigned result = 5381; > > + while (char c = *string++) > > + result = result * 33 + c; > > + return result; > > +} > > Nits: you can make this `constexpr` function, move it to a header, and > calculate hash value at compile time. Really, the compiler will execute a constexpr loop?
Filip Pizlo
Comment 6 2018-03-11 10:18:50 PDT
Created attachment 335540 [details] patch for landing With Yusuke's suggested change.
EWS Watchlist
Comment 7 2018-03-11 10:20:44 PDT
Attachment 335540 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/PerProcess.h:72: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/PerProcess.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 2 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 8 2018-03-11 10:21:23 PDT
(In reply to Filip Pizlo from comment #5) > > Nits: you can make this `constexpr` function, move it to a header, and > > calculate hash value at compile time. > > Really, the compiler will execute a constexpr loop? Yeah, C++14 relaxed constexpr is super powerful. You can check our wtf/text/StringHasher.h's computeLiteralHash for example.
Filip Pizlo
Comment 9 2018-03-11 10:46:22 PDT
Landed in https://trac.webkit.org/changeset/229516/webkit
Radar WebKit Bug Importer
Comment 10 2018-03-12 17:24:43 PDT
<rdar://problem/38397867>
Joseph Pecoraro
Comment 11 2020-02-05 11:40:08 PST
*** Bug 182496 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.