Bug 175809

Summary: Make generate_offset_extractor.rb architectures argument more robust
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, joepeck, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch joepeck: review+

Keith Miller
Reported 2017-08-21 19:57:13 PDT
Make generate_offset_extractor.rb architetures argument more robust
Attachments
Patch (1.69 KB, patch)
2017-08-21 19:59 PDT, Keith Miller 		joepeck: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Keith Miller
Comment 1 2017-08-21 19:59:23 PDT
Created attachment 318726 [details] Patch
Joseph Pecoraro
Comment 2 2017-08-21 20:04:16 PDT
Comment on attachment 318726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318726&action=review > Source/JavaScriptCore/ChangeLog:3 > + Make generate_offset_extractor.rb architetures argument more robust Typo in bug name: architetures > Source/JavaScriptCore/offlineasm/generate_offset_extractor.rb:42 > -validBackends = canonicalizeBackendNames(ARGV.shift.split(",")) > +validBackends = canonicalizeBackendNames(ARGV.shift.split(/,+\s*|\s+/)) How about: /[,\s]+/ This to require any combination of spaces and commas. Your regex supports leading commands and trailing whitespace but not emptiness in-between, which seems weird.
Keith Miller
Comment 3 2017-08-21 20:06:57 PDT
Comment on attachment 318726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318726&action=review >> Source/JavaScriptCore/ChangeLog:3 >> + Make generate_offset_extractor.rb architetures argument more robust > > Typo in bug name: architetures Whoops fixed. >> Source/JavaScriptCore/offlineasm/generate_offset_extractor.rb:42 >> +validBackends = canonicalizeBackendNames(ARGV.shift.split(/,+\s*|\s+/)) > > How about: > > /[,\s]+/ > > This to require any combination of spaces and commas. Your regex supports leading commands and trailing whitespace but not emptiness in-between, which seems weird. Yeah, that's a better Regex! I'll change.
Keith Miller
Comment 4 2017-08-21 20:19:55 PDT
Committed r221000: <http://trac.webkit.org/changeset/221000>
Radar WebKit Bug Importer
Comment 5 2017-08-21 20:20:57 PDT
<rdar://problem/34006009>
Note You need to log in before you can comment on or make changes to this bug.