std::os::unix::fs::MetadataExt::size() silently truncates file size on Android

[eefriedman]

The implementation of size() looks like this:

fn size(&self) -> raw::off_t { self.as_raw_stat().st_size as raw::off_t }

The field st_size has type c_longlong, and the type raw::off_t has type i32 on 32-bit Android. The combination doesn't work well.

[alexcrichton]

There's not really a whole lot we can do here. Options are:

1. Have a different trait definition on Android
2. Create our own typedef which does the "right thing" on android
3. Closes as WONTFIX and say that the raw as_raw_stat should be used in situations like this instead.

[eefriedman]

Making raw::off_t an i64 is probably the least bad solution; it probably matches what we will do on Linux. See https://internals.rust-lang.org/t/pre-rfc-rust-and-large-file-support-lfs/2777 .

[alexcrichton]

Unfortunately off_t on Android is defined to be a long which I believe is i32

[eefriedman]

There isn't any hard requirement that raw::off_t has to be equivalent to the C off_t (or libc::off_t). Granted, it might be kind of confusing.

[alexcrichton]

I would personally be worried about someone binding a C API that looked like:

off_t foo(ptr_t *p);

Where when transcribed into Rust with the raw module it wouldn't actually be correct. This is where defining our own typedef, though, may not be the end of the world.

[cuviper]

Are you sure Android off_t is only long?

It sure looks like bionic's sys/types.h makes off_t configuration dependent:

#if defined(__USE_FILE_OFFSET64) || defined(__LP64__)
typedef int64_t off_t;
typedef off_t loff_t;
typedef loff_t off64_t;
#else
/* This historical accident means that we had a 32-bit off_t on 32-bit architectures. */
typedef __kernel_off_t off_t;
typedef __kernel_loff_t loff_t;
typedef loff_t off64_t;
#endif

Where __USE_FILE_OFFSET64 is defined over in cdef.h by the user's _FILE_OFFSET_BITS.

FWIW, I confirmed on my Nexus 9, lib/libc.so does have variants covering off_t differences, e.g. lseek and lseek64. (That's the 32-bit ARM library, vs. lib64/libc.so for aarch64.) It's nice that struct stat is defined in an always-64bit manner though.

So I wouldn't worry about someone binding some other C API with off_t -- that's already fraught with danger. I think this falls exactly into my LFS goal that @eefriedman linked on internals.

[eefriedman]

Note that Android only got __USE_FILE_OFFSET64 recently, and it has limitations; see https://android.googlesource.com/platform/bionic/+/68dc20d41193831a94df04b994ff2f601dd38d10%5E!/

[cuviper]

Ah, that's too bad. At least they've had most of the explicit LFS functions for a while, around late 2010, as well as 64-bit loff_t and off64_t. And I still maintain it's a bad idea for others to be using off_t in a C API.

[alexcrichton]

Ah yeah the headers I was looking at didn't have these #defines (something about android version 18?). Although it's probably not the end of the world on Android as a platform just yet!

[alexcrichton]

I've now opened an RFC which I hope will help enable us to fix this issue in a "backwards-compatible" fashion