Skip to content
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

Fix build on aarch64 #13

Merged
merged 1 commit into from
Dec 26, 2023

Conversation

mfrischknecht
Copy link
Contributor

@mfrischknecht mfrischknecht commented Dec 21, 2023

attach_to_tmux_session_outside_tmux uses raw pointers to c_char under the hood, but these were specified explicitly as *const i8 instead of *const c_char. As the documentation for CString::as_ptr 1 points out, the method really returns a *const c_char, and the actual referenced byte type (i8 or u8) is architecture dependent:

The type of the returned pointer is *const c_char, and whether it’s an
alias for *const i8 or *const u8 is platform-specific.

Changing the pointer type to *const c_char explicitly fixes the build on aarch64, where the underlying pointer type is actually *const u8 instead of *const i8.

Footnotes

  1. https://doc.rust-lang.org/std/ffi/struct.CStr.html#method.as_ptr

`attach_to_tmux_session_outside_tmux` uses raw pointers to `c_char` under
the hood, but these were specified explicitly as `*const i8` instead of
`*const c_char`. As the documentation for `CString::as_ptr` [1] points
out, the method really returns a `*const c_str`, and the actual referenced
byte type (`i8` or `u8`) is architecture dependent:

> The type of the returned pointer is *const c_char, and whether it’s an
> alias for *const i8 or *const u8 is platform-specific.

Changing the pointer type to `*const c_char` explicitly fixes the build
on aarch64, where the underlying pointer type is actually `*const u8`
instead of `*const i8`.

[1]: https://doc.rust-lang.org/std/ffi/struct.CStr.html#method.as_ptr
@mfrischknecht
Copy link
Contributor Author

@vinnymeller

I was sifting through failing Nixpkgs builds hoping to help fix a couple of Hydra build failures. This change was the result of investigating this build: https://hydra.nixos.org/build/242898606

Since you're also registered as the maintainer of the twm Nix package, I thought, simply posting the patch on Nixpkgs would be a little bit nonsensical.

The relevant log output from Hydra looks like this:

   Compiling twm v0.8.0 (/build/source)
error[E0277]: a value of type `Vec<*const i8>` cannot be built from an iterator over elements of type `*const u8`
   --> src/tmux.rs:108:10
    |
108 |         .collect();
    |          ^^^^^^^ value of type `Vec<*const i8>` cannot be built from `std::iter::Iterator<Item=*const u8>`
    |
    = help: the trait `FromIterator<*const u8>` is not implemented for `Vec<*const i8>`
    = help: the trait `FromIterator<T>` is implemented for `Vec<T>`
note: the method call chain might not have had the expected associated types
   --> src/tmux.rs:106:10
    |
97  |       let tmux_attach_args = vec![
    |  ____________________________-
98  | |         CString::new("tmux").unwrap(),
99  | |         CString::new("attach").unwrap(),
100 | |         CString::new("-t").unwrap(),
101 | |         CString::new(repo_name).with_context(|| "Unable to turn repo name to a cstring.")?,
102 | |     ];
    | |_____- this expression has type `Vec<CString>`
...
105 |           .iter()
    |            ------ `Iterator::Item` is `&CString` here
106 |           .map(|arg| arg.as_ptr())
    |            ^^^^^^^^^^^^^^^^^^^^^^^ `Iterator::Item` changed to `*const u8` here
107 |           .chain(std::iter::once(std::ptr::null()))
    |            ---------------------------------------- `Iterator::Item` remains `*const u8` here
note: required by a bound in `std::iter::Iterator::collect`
   --> /build/rustc-1.74.0-src/library/core/src/iter/traits/iterator.rs:2049:5

error[E0308]: mismatched types
   --> src/tmux.rs:111:38
    |
111 |         execvp(tmux_attach.as_ptr(), tmux_attach_args_ptrs.as_ptr());
    |         ------                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `*const *const u8`, found `*const *const i8`
    |         |
    |         arguments to this function are incorrect
    |
    = note: expected raw pointer `*const *const u8`
               found raw pointer `*const *const i8`
note: function defined here
   --> /build/twm-0.8.0-vendor.tar.gz/libc/src/unix/mod.rs:892:12
    |
892 |     pub fn execvp(c: *const c_char, argv: *const *const c_char) -> ::c_int;
    |            ^^^^^^

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
error: could not compile `twm` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...

@vinnymeller
Copy link
Owner

@mfrischknecht thank you for this!

@vinnymeller vinnymeller merged commit a4f2f8d into vinnymeller:master Dec 26, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants