Summary: | PerProcess<> should be safe by default | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||
Component: | bmalloc | Assignee: | 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
Filip Pizlo
2018-03-10 13:19:47 PST
Created attachment 335519 [details]
maybe this will work
Created attachment 335520 [details]
the patch
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.
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. (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? Created attachment 335540 [details]
patch for landing
With Yusuke's suggested change.
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.
(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. *** Bug 182496 has been marked as a duplicate of this bug. *** |