Extend supported range requests for large files on 64-bit hosts

[jonathonf]

This extends the supported range request sizes and thereby allows download of larger files on 64-bit hosts. Based on initial work by @ghuls (within #31 comments) and @TotallyNotElite (JustTNE@aa5d024)

[TheAssassin]

What's the type of buffer_pos?

You should use a fixed 64-bit type like uint64_t, or unsigned long long. On 32-bit architectures, size_t is 4 bytes in size.

[jonathonf]

buffer_pos is size_t: https://github.com/AppImage/zsync2/blob/86cfd3a1d6a27483ec40edd62c1a6bd409cbbe5d/src/legacy_http.c#L78

[TheAssassin]

All these need to have a real 8-byte size type like uint64_t. I'm very certain size_t will be too small on 32-bit, same goes for off_t. I just tried with gcc -m32:

#include <stdio.h>
#include <sys/types.h>

int main() {
    printf("%lu\n", sizeof(off_t));
}

It's generally a good idea to use fixed-size types, as they're predictable and not compiler-dependent. And in this case, it's even a must, since we also have 32-bit binaries. You can't support big files on 64-bit only.

[TheAssassin]

That is, by the way, the big issue with the patch in #31.

[jonathonf]

It appears that just opening a 2GB+ file with a 32-bit build is currently broken, e.g. at

https://github.com/AppImage/zsync2/blob/86cfd3a1d6a27483ec40edd62c1a6bd409cbbe5d/src/zsclient.cpp#L986

so just increasing type size on those systems won't help.

[TheAssassin]

Could be. But I don't see the point in accepting a pull request that doesn't completely solve an issue, while it claims to do so. Renaming the PR doesn't make things better, really.

[jonathonf]

The PR was renamed specifically because you pointed out that it doesn't actually solve the whole issue, yet does still improve things for 64-bit builds (i.e. a 64-bit build can download files larger than 2GB, which, for me at least, makes things better than when #31 was opened in 2018).

[TheAssassin]

The argument is nonsense. There is no reason not to just swap the types you change anyway for ones that actually work on 32-bit as well. It doesn't have to make 32-bit work entirely... Nobody requested this.

But your PR is really off-putting in that its argument is "at least it's better than nothing". Of course, a full-featured 32-bit fix would be nice. But I'm certain you won't be willing to work on that. At least you could fix the types in the lines you touch anyway, permanently.

[jonathonf]

It doesn't have to make 32-bit work entirely... Nobody requested this

I may have misinterpreted this statement:

I don't see the point in accepting a pull request that doesn't completely solve an issue

[TheAssassin]

My point is, if you know an exact set of lines has to be touched in the future because the data types don't work on one platform properly, it really makes sense to just use the future proof data type. I basically had to re-do your changes to change the types to the future proof ones. This resulted in twice the work overall.

[TheAssassin]

I did it myself now, in 1 minute plus 2 minutes of committing. Thanks for bringing this issue up on my desk again, at least.

[jonathonf]

Great, thanks. Good to see some incremental improvements.

[ghuls]

For opening large file with a 32-bit binary, you need to enable 32-bit support large file support.
This should at least work with g++ on Linux.

Add the option -D_LARGE_FILE_SOURCE=1 to gcc compilation.

fseek64() is a C function. To make it available you'll have to define _FILE_OFFSET_BITS=64 before including the system headers. That will more or less define fseek to behave as actually fseek64. Or you could do it in the compiler arguments e.g. gcc -D_FILE_OFFSET_BITS=64, that you are already doing.

Compile your programs with gcc -D_FILE_OFFSET_BITS=64. This forces all file access calls to use the 64 bit variants. Several types change also, e.g. off_t becomes off64_t. It's therefore important to always use the correct types and to not use e.g. int instead of off_t in your C code. For portability with other platforms you should use getconf LFS_CFLAGS which will return -D_FILE_OFFSET_BITS=64 on Linux platforms but might return something else on for e.g. on Solaris. For linking, you should use the link flags that are reported via getconf LFS_LDFLAGS. On Linux systems, you do not need special link flags. Define _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE. With these defines you can use the LFS functions like open64 directly. Use the O_LARGEFILE flag with open to operate on large files.

https://stackoverflow.com/questions/14533836/large-file-support-not-working-in-c-programming

https://stackoverflow.com/questions/965725/large-file-support-in-c