Bug 6900 - apply Vector class to KURL
Summary: apply Vector class to KURL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Maciej Stachowiak
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-01-28 20:46 PST by Maciej Stachowiak
Modified: 2006-01-29 19:46 PST (History)
0 users

See Also:


Attachments
apply Vector to KURL (12.44 KB, patch)
2006-01-28 21:09 PST, Maciej Stachowiak
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Stachowiak 2006-01-28 20:46:38 PST
KURL should use Vector for its stack buffers, and in place of Array.
Comment 1 Maciej Stachowiak 2006-01-28 21:09:06 PST
Created attachment 6062 [details]
apply Vector to KURL

I ran performance tests and found no regression. Also all the layout tests pass.
Comment 2 Darin Adler 2006-01-28 21:14:10 PST
Comment on attachment 6062 [details]
apply Vector to KURL

Looks good. r=me
Comment 3 David Kilzer (:ddkilzer) 2006-01-29 06:09:08 PST
Comment on attachment 6062 [details]
apply Vector to KURL

>Index: kwq/KWQKURL.mm
>===================================================================
>--- kwq/KWQKURL.mm	(revision 12427)
>+++ kwq/KWQKURL.mm	(working copy)
>@@ -238,15 +231,9 @@ KURL::KURL() : m_isValid(false)
> KURL::KURL(const char *url)
> {
>     if (url && url[0] == '/') {
>-        char staticBuffer[2048];
>-        char *buffer;
>+         // 5 for "file:", 1 for terminator
>         size_t urlLength = strlen(url) + 1;
>-        size_t bufferLength = urlLength + 5; // 5 for "file:"
>-        if (bufferLength > sizeof(staticBuffer)) {
>-            buffer = (char *)fastMalloc(bufferLength);
>-        } else {
>-            buffer = staticBuffer;
>-        }
>+        Vector<char, 2048> buffer(urlLength + 5);
>         buffer[0] = 'f';
>         buffer[1] = 'i';
>         buffer[2] = 'l';

Shouldn't the "5" be a "6" in the new Vector declaration above?

Also, it seems that every time that Vector is used, a "1" must be explicitly added for the terminator character.  Wouldn't it be best to pull this behavior back into the template class and automatically add 1 for the caller?
Comment 4 David Kilzer (:ddkilzer) 2006-01-29 06:09:28 PST
Reopening per Comment #3.
Comment 5 David Kilzer (:ddkilzer) 2006-01-29 19:46:13 PST
(In reply to comment #3)
> Shouldn't the "5" be a "6" in the new Vector declaration above?

FYI, the "+ 1" is added on the line above, so this isn't an issue.