[Webkit-unassigned] [Bug 117735] [curl] Improve multipart response handling

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=117735





--- Comment #10 from Christophe Dumez <dchris at gmail.com>  2013-08-14 04:28:11 PST ---
(From update of attachment 208710)
View in context: https://bugs.webkit.org/attachment.cgi?id=208710&action=review

> Source/WebCore/platform/network/curl/MultipartHandle.cpp:63
> +        boundaryEnd = contentType.find(";", boundaryStart);

';'

> Source/WebCore/platform/network/curl/MultipartHandle.cpp:77
> +bool MultipartHandle::matchForBoundary(const char* data, size_t pos, size_t& matchedLength)

We don't use abbreviations in WebKit: pos -> position

> Source/WebCore/platform/network/curl/MultipartHandle.cpp:82
> +        if (data[pos + i] != m_boundary[i]) {

how do we know data[pos + i] does not go out of bounds?

> Source/WebCore/platform/network/curl/MultipartHandle.cpp:92
> +bool MultipartHandle::checkForBoundary(size_t& boundaryStartPos, size_t& lastPartialMatchPos)

"Pos" -> "Position'

> Source/WebCore/platform/network/curl/MultipartHandle.cpp:99
> +    lastPartialMatchPos = contentLength;

Ditto.

> Source/WebCore/platform/network/curl/MultipartHandle.cpp:117
> +    const char* content = m_buffer.data();

This can be moved after the if check for contentLength.

> Source/WebCore/platform/network/curl/MultipartHandle.cpp:135
> +    char* end = const_cast<char*>(content) + contentLength;

Cannot this one be const?

> Source/WebCore/platform/network/curl/MultipartHandle.cpp:249
> +        if (!m_buffer.size())

m_buffer.isEmpty()

> Source/WebCore/platform/network/curl/MultipartHandle.h:77
> +    Vector<char> m_buffer;

Considering the operations you are doing (append and remove from beginning), a Vector may not be the best data structure. You might want to take a look at StreamBuffer (or Deque)

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:406
> +                d->m_multipartHandle = adoptPtr(new MultipartHandle(job, boundary));

We usually use static factory functions such as PassOwnPtr<MultipartHandle> MultipartHandle::create() for such cases so that we never deal with raw pointers externally.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.