android 32 bits fix stat struct proposal.

[devnexen]

close #3285

[rustbot]

r? @JohnTitor

(rustbot has picked a reviewer for you, use r? to override)

[JohnTitor]

@bors r+

[bors]

📌 Commit 57bfcd8 has been approved by JohnTitor

It is now in the queue for this repository.

[bors]

⌛ Testing commit 57bfcd8 with merge 3fac855...

[bors]

💔 Test failed - checks-actions

[tgross35]

Looks like this doesn't agree with the libc we are checking against. I think @danielocfb was saying that we should use e.g. dev_t rather than c_ulong

@rustbot author

[tgross35]

@maurer would you mind looking at this? This would be a breaking change to stat for libc 1.0.

[maurer]

This change is incorrect - the bionic stat definition looks like the left hand side for 32-bit platforms.

I can comment over on the issue, but the reason this one is different is because the actual value returned here is not an ino_t. Even outside bionic (which you should use as the source of truth for what Android libc structures look like), 32-bit ARM musl has a similar header which makes the fact that this field is truncated more explicit.

It looks like there may be a couple places where using the "right type" is plausible (e.g. musl thinks that using dev_t for the first one could work), and I could talk to the bionic maintainer about making this switch if it would be helpful, but the proposed change would actually just flat out work incorrectly, the new proposed structure has the wrong offsets.

[tgross35]

It's been a while, @devnexen would you be able to make this closer match the Android definitions as the maintainer requested? Looks like stat64 could also be made an alias since they have identical definitions (this may have already happened).

[rustbot]

Some changes occurred in an Android module

cc @maurer

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[rustbot]

⚠️ Warning ⚠️

• There are uncanonicalized issue links (such as #123) in the commit messages of the following commits.
  Please add the organization and repository before the issue number (like so rust-lang/rust#123) to avoid issues with subtree.
  • 0a07719

[tgross35]

Seems fine to me now, @maurer could you take a look?

See also #5069 for changing to the timespec types eventually

[tgross35]

What's the status of 64-bit time on 32-bit Android actually, is that a supported configuration? Based on https://cs.android.com/android/platform/superproject/+/android-latest-release:bionic/libc/include/sys/types.h;l=97-99;drc=0d34bb1a4ce70114d96ca50aab8012b571d4e7c0 and https://cs.android.com/android/platform/superproject/+/android-latest-release:external/kernel-headers/original/uapi/asm-generic/posix_types.h;l=92;drc=f2abbe6d3587e83f95b5fa7f9da6e4a1f981ad38, I don't see any indication that it is.

Cc @dybucc since you may know or be interested in the answer.

[maurer]

Looks fine now.

[tgross35]

Thanks!

View changes since this review

[dybucc]

What's the status of 64-bit time on 32-bit Android actually, is that a supported configuration? Based on https://cs.android.com/android/platform/superproject/+/android-latest-release:bionic/libc/include/sys/types.h;l=97-99;drc=0d34bb1a4ce70114d96ca50aab8012b571d4e7c0 and https://cs.android.com/android/platform/superproject/+/android-latest-release:external/kernel-headers/original/uapi/asm-generic/posix_types.h;l=92;drc=f2abbe6d3587e83f95b5fa7f9da6e4a1f981ad38, I don't see any indication that it is.

Cc @dybucc since you may know or be interested in the answer.

What little I know comes from this document ¹. There's also info there on
32-bit file offset types.

Footnotes

1. https://android.googlesource.com/platform/bionic/+/main/docs/32-bit-abi.md#is-32_bit-on-lp32-y2038 ↩

[tgross35]

Linux 5.x kernels do offer extra interfaces so that 32-bit processes can pass 64-bit times to/from the kernel, but we do not plan on adding support for these to the C library

Sounds like there's nothing for us to support then - that makes it easy.