[webkit-reviews] review requested: [Bug 17971] [Curl] FormData processing should be moved to its own class : [Attachment 20992] Second version (address Eric's comments)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 6 17:27:02 PDT 2008


Julien Chaffraix <julien.chaffraix at gmail.com> has asked  for review:
Bug 17971: [Curl] FormData processing should be moved to its own class
http://bugs.webkit.org/show_bug.cgi?id=17971

Attachment 20992: Second version (address Eric's comments)
http://bugs.webkit.org/attachment.cgi?id=20992&action=edit

------- Additional Comments from Julien Chaffraix <julien.chaffraix at gmail.com>
> My only question was if the ownership was right for
> ResourceHandle*, i.e. why is it safe to hold a weak pointer?

Checked and added a comment about why a weak pointer is alright (basically we
can have that since the ResourceHandle has a strong reference on its
FormDataStream).

> Using FILE*
> directly kinda sucks too.  But I guess we don't have any nice C++ wrapper for

> it.
I am afraid we will have to use FILE* until it exists.


> integer overflow?  We really should have some sort of safe-multiply function.


Added an overflow check.

> What the heck does "nmemb" stand for?  A more clear variable name would be
> helpful here.

Number of MEMory Block is my guess. It comes from libcURL documentation.
Updated the name in FormDataStream but left it in ResourceHandleManager
callback as IMHO it is more intuitive there.

Corrected the coding style issues.


More information about the webkit-reviews mailing list