Bug 210519

Summary: Use CFArrayGetValues() in createArchiveList() in WebCoreArgumentCodersMac.mm
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: WebKit2Assignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=210456
Bug Depends on: 210448    
Bug Blocks:    
Attachments:
Description Flags
Patch v1 darin: review+

Description David Kilzer (:ddkilzer) 2020-04-14 15:48:08 PDT
Use CFArrayGetValues() in createArchiveList() in WebCoreArgumentCodersMac.mm.

Via Darin Adler in Bug 210448, Comment #4:

Separately, I suggest we rewrite this using CFArrayGetValues, rather than calling CFArrayGetValueAtIndex over and over again in a loop. We will still have to loop over the buffer to convert tokenNull into nullptr, but it should be more efficient to do it that way rather than call CFArrayGetValueAtIndex over and over again.
Comment 1 David Kilzer (:ddkilzer) 2020-04-14 16:07:45 PDT
Created attachment 396472 [details]
Patch v1
Comment 2 Darin Adler 2020-04-14 16:23:54 PDT
Comment on attachment 396472 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=396472&action=review

> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:127
> +    for (size_t i = 0; i < static_cast<size_t>(*objectCount); ++i) {

This line should not have been changed. There’s no reason to cast this just so we can use a different type for the loop. CFIndex is fine.
Comment 3 David Kilzer (:ddkilzer) 2020-04-14 18:04:16 PDT
Comment on attachment 396472 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=396472&action=review

>> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:127
>> +    for (size_t i = 0; i < static_cast<size_t>(*objectCount); ++i) {
> 
> This line should not have been changed. There’s no reason to cast this just so we can use a different type for the loop. CFIndex is fine.

Will change back.  I seem to recall there's a clang warning about using a signed type for an array index, which is why I changed it.
Comment 4 David Kilzer (:ddkilzer) 2020-04-14 18:08:51 PDT
Committed r260111: <https://trac.webkit.org/changeset/260111>
Comment 5 Radar WebKit Bug Importer 2020-04-14 18:09:15 PDT
<rdar://problem/61800359>