Skip to content

Revert of mut api changes breaking rust userspace on AIX #4544

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

entd
Copy link

@entd entd commented Jul 9, 2025

Description

Reverted C api changes introduced by commit c192a5c

IBM AIX does not define pointer constness traditionally in their headers. Some commit changed const pointers to mutable ones, those breaking many userspace libraries, as those in rust libc are usually defined on other posix platforms as const.

Some older commit (mainly c192a5c) changed the api.
I reverted fstat, swapon apis, to expose path variable pointer as const again.

Sources

https://www.ibm.com/docs/en/aix/7.2.0?topic=s-stat-fstat-lstat-statx-fstatx-statxat-fstatat-fullstat-ffullstat-stat64-fstat64-lstat64-stat64x-fstat64x-lstat64x-stat64xat-subroutine)

Checklist

  • [x ] Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot label +stable-nominated

@rustbot rustbot added O-unix stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Jul 9, 2025
@entd entd changed the title Revert of mut api breaking userspace users on AIX Revert of mut api changes breaking rust userspace on AIX Jul 9, 2025
@entd entd marked this pull request as ready for review July 9, 2025 14:58
@tgross35
Copy link
Contributor

tgross35 commented Jul 9, 2025

This is tricky. We really need to match the platform API for forward compatibility, but at the same time it would be pretty awful if statfs started mutating path.

Cc @xingxue-ibm, @gilamn5tr since #4450 is the referenced commit.

IBM AIX does not define pointer constness traditionally in their headers.

Is this something in the process of changing?

@entd
Copy link
Author

entd commented Jul 10, 2025

I am just third party user not affiliated to the IBM. Maybe IBM maintainers here could tell better. I think it might be leftover from pre ANSI C, not that those headers are require non const pointers in C.

For me biggest problem, that many rust user libs expect consts here. Like simple networking libraries using c:statfs path fails because AIX backend in this library his does not wrap const* and expose mut*.
As for forward compatibility I think mut* can be safely coerced to const* and those there should not be huge issues.
The real decider should be: does AIX internally handle those path pointers as mutable or it is just leftover and mess from the pre-ANSI days.
If actually those are indeed need to be mutable we probably should wrap the function with string copy to have consistent api for external libraries. I feel this change should be in this library, not offloaded to the huge userspace which now would have write absurd amount AIX specific code just to accomodate one mut* not maching most of POSIX world.

Maybe @xingxue-ibm OP of the original commit can guide us here?

@tgross35
Copy link
Contributor

To clarify: the best case scenario here is that AIX maintainers (1) confirm the arguments can safely be pointers to read-only data, and (2) are willing to update the documentation / headers to reflect that. const char * Path is used for stat on the doc page linked in the top post so I would assume statx could/should be similar.

I am willing to merge this PR with only the maintainer confirmation, the docs update would just be nice to make things more clear for the future (and of course to help other consumers of those docs).

If actually those are indeed need to be mutable we probably should wrap the function with string copy to have consistent api for external libraries. I feel this change should be in this library, not offloaded to the huge userspace which now would have write absurd amount AIX specific code just to accomodate one mut* not maching most of POSIX world.

This would not happen, libc provides bindings but does not abstract over platform differences. There are other libraries such as nix that provide higher-level interfaces.

@entd
Copy link
Author

entd commented Jul 10, 2025

@tgross35 OK, I do not question policy of this library, anyway still it would be nice to know what is actually going here from AIX api side. I found this issue from higher level wrapper (namely rustix) and if that the case mutable is needed here, these platform differences could be resolved at those higher level libraries.

Actually I am trying to extend rustix cover aix. But this const -> mut thing caught my eye on libc.

I would prefer that api would not be mutable only because some obscure historical reason without the real need of mutability.

@entd
Copy link
Author

entd commented Jul 11, 2025

There are worse type mismatches than consts, example ioctls are broken as constants now are long, but ioctl symbol expects int...:

    --> /home/ibrasiskis/.cargo/registry/src/index.crates.io-7f555b6b8ccf4919/jobserver-0.1.33/src/unix.rs:356:57
     |
356  |         cvt(unsafe { libc::ioctl(self.read.as_raw_fd(), libc::FIONREAD, len.as_mut_ptr()) })?;
     |                      -----------                        ^^^^^^^^^^^^^^ expected `i32`, found `i64`
     |                      |
     |                      arguments to this function are incorrect
     |
note: function defined here
    --> /home/ibrasiskis/.cargo/registry/src/index.crates.io-7f555b6b8ccf4919/libc-0.2.174/src/unix/aix/mod.rs:2994:12
     |
2994 |     pub fn ioctl(fildes: c_int, request: c_int, ...) -> c_int;
     |            ^^^^^
help: you can convert an `i64` to an `i32` and panic if the converted value doesn't fit
     |
356  |         cvt(unsafe { libc::ioctl(self.read.as_raw_fd(), libc::FIONREAD.try_into().unwrap(), len.as_mut_ptr()) })?;
     |                                                                       ++++++++++++++++++++

For more information about this error, try `rustc --explain E0308`.```

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-unix S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants