[webkit-reviews] review denied: [Bug 98055] [soup] WebKit crashes when doing a http request : [Attachment 166910] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied arno. <arno at renevier.net>'s
request for review:
Bug 98055: [soup] WebKit crashes when doing a http request
https://bugs.webkit.org/show_bug.cgi?id=98055

Attachment 166910: Patch
https://bugs.webkit.org/attachment.cgi?id=166910&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=166910&action=review


Okay. Seems reasonable, but consider the following. Please also follow the
suggestion above.

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:379
> +	   g_object_set_data(G_OBJECT(d->m_soupMessage.get()), "handle", NULL);


Use 0 instead of NULL here.

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:769
> -	   if (d->m_firstRequest.timeoutInterval() > 0) {
> +	   guint timeoutInterval = d->m_firstRequest.timeoutInterval() * 1000;
> +	   if (timeoutInterval > 0) {

See below.

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:878
> +	   guint timeoutInterval = d->m_firstRequest.timeoutInterval() * 1000;
> +	   if (timeoutInterval > 0) {

Maybe it would be clearer to simply check for INT_MAX explicitly and just leave
a comment like:

// INT_MAX * 1000 is zero on i386, so we explicitly check this situation.
unsigned timeoutInterval = d->m_firstRequest.timeoutInterval();
if (timeoutInterval && timeoutInterval != INT_MAX) {

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:1045
> +	   guint timeoutInterval = d->m_firstRequest.timeoutInterval() * 1000;
> +	   if (timeoutInterval > 0) {

Ditto.