On Tuesday, December 29, 2009, at 01:29PM, "Steven M. Schweda" <sms_at_antinode.info> wrote:
>From: Yang Tse <yangsita_at_gmail.com>
>
>> > http://antinode.info/ftp/curl/curl-7_19_7/curl-7_19_7_s1.dru
>>
>> I've looked only into the initial part of that patch...
>>
>> Once more quoting note 2 of curlbuild.h.dist...
>> [...]
>> So pleas, explain the reasons to absolutely ignore the above and gate
>> CURL_TYPEOF_CURL_OFF_T to __off_t in your patch.
>
> Primarily, I couldn't see any reason for the above. All the
>underlying I/O functions expect off_t arguments, so what's the point in
>trying to find some non-off_t type which exactly matches off_t, when
>off_t would seem to do the job perfectly?

I don't think they want one that matches off_t. It think they want one that is 64 bits regardless of what off_t is.

> It would be ok with me to switch it back to using "long long"
>(non-VAX and _LARGEFILE) or "int" (VAX or not _LARGEFILE), so long as
>the conditionality looks more like
> # if !defined(__VAX) && defined(_LARGEFILE)
>than
> # if defined(__alpha) || defined(__ia64)
>which is simply wrong.

It's wrong if the expectation is that CURL_TYPEOF_CURL_OFF_T is the same size as off_t, but that seems not to be what's expected, which probably has a reason, though I didn't see any stated in the comments quoted from curlbuild.h.dist.

> One advantage of using the method I chose is that it pretty well
>guarantees that CURL_TYPEOF_CURL_OFF_T is consistent with the actual
>type of off_t, or else the compiler will let you know about it.

For some reason the maintainers appear to want to guarantee the opposite of that, i.e. that CURL_TYPEOF_CURL_OFF_T has 64 bits even when off_t has only 32 bits. My guess would be that for binary compatibility reasons they want the libcurl interfaces to all use 64-bit integers even when off_t is only 32 bits. In theory you could upgrade a libcurl shared library built this way that is not large-file-capable with one that is and not have to change or recompile your code. That assumes that the libcurl code all does appropriate range checking or downgrading the 64-bit ints to 32-bit if necessary. I'm not familiar with the code so I don't know if that happens.

FWIW I've read through Steven's patch and it all looks good and makes sense to me with the possible exception of this one nit about 64-bitness. I suspect we'll get that ironed out with a bit more clarification.

I'm going to do a build with the patch and report back.

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2009-12-29