[webkit-reviews] review denied: [Bug 18832] [curl] file upload does not work : [Attachment 23876] Pass an integer of the right size based on curl's compile options

[bugzilla-daemon at webkit.org]

Alp Toker <alp at nuanti.com> has denied Marco Barisione
<marco.barisione at collabora.co.uk>'s request for review:
Bug 18832: [curl] file upload does not work
https://bugs.webkit.org/show_bug.cgi?id=18832

Attachment 23876: Pass an integer of the right size based on curl's compile
options
https://bugs.webkit.org/attachment.cgi?id=23876&action=edit

------- Additional Comments from Alp Toker <alp at nuanti.com>
I agree this change is necessary since we have no control over which versions
of libraries distributors will link against.

>Index: WebCore/platform/network/curl/ResourceHandleManager.cpp
>===================================================================
>--- WebCore/platform/network/curl/ResourceHandleManager.cpp	(revisione
36996)
>+++ WebCore/platform/network/curl/ResourceHandleManager.cpp	(copia locale)
>@@ -3,6 +3,7 @@
>  * Copyright (C) 2006 Michael Emmel mike.emmel at gmail.com
>  * Copyright (C) 2007 Alp Toker <alp.toker at collabora.co.uk>
>  * Copyright (C) 2007 Holger Hans Peter Freyther
>+ * Copyright (C) 2008 Collabora Ltd.
>  * All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>@@ -377,12 +378,24 @@
>     }
> 
>     // Obtain the total size of the POST
>+    // The size of a curl_off_t could be different in WebKit and in cURL
depending on
>+    // compilation flags of both. For CURLOPT_POSTFIELDSIZE_LARGE we have to
pass the
>+    // right size or random data will be used as the size.
>+    int expectedSizeOfCurlOffT = 0;
>+    if (!expectedSizeOfCurlOffT) {
>+	  curl_version_info_data *infoData =
curl_version_info(CURLVERSION_NOW);
>+	  if (infoData->features & CURL_VERSION_LARGEFILE)
>+	      expectedSizeOfCurlOffT = sizeof(long long);
>+	  else
>+	      expectedSizeOfCurlOffT = sizeof(int);
>+    }
>+

Looks like there's something wrong above, since !expectedSizeOfCurlOffT will
always be true.

perhaps 'int expectedSizeOfCurlOffT' should be static?

I won't be able to test this one against different versions of curl myself but
I'm guessing Marco has already tested it.

Looks good otherwise.