dcuddeback / termios-rs

Safe bindings for the termios library.
http://dcuddeback.github.io/termios-rs/termios/
MIT License
75 stars 25 forks source link

Add android support #15

Closed spk closed 5 years ago

spk commented 6 years ago

This will allow to install bat on android Cheers

For CI fix see https://github.com/dcuddeback/termios-rs/pull/16

dcuddeback commented 5 years ago

@spk Thanks. I'm not very familiar with Android myself. Do you have some instructions that I could follow to test this? I'm happy to merge as long as I can verify the fix myself.

spk commented 5 years ago

Hi @dcuddeback sure there is a good documentation here https://blog.rust-lang.org/2016/05/13/rustup.html#example-running-rust-on-android

But you can check with:

rustup target add arm-linux-androideabi
cargo build --target=arm-linux-androideabi

On master this will fail with:

error[E0433]: failed to resolve. Could not find `target` in `os`

On this branch it should compile

YtvwlD commented 5 years ago

This branch works for me, too.

Here are some steps to reproduce: (They are less convenient, but perhaps more realistic.)

  1. Get an Android device or an emulator (the official one or Genymotion)
  2. Install Termux (from Google Play or F-Droid)
  3. Add a repository containing cargo and rustc: curl https://its-pointless.github.io/setup-pointless-repo.sh | sh
  4. Install cargo and rustc (and git): apt update && apt install git cargo rustc
  5. Test
dcuddeback commented 5 years ago

@spk @YtvwlD I started looking at this by comparing the definitions to those found in the header files in the Android NDK from @spk's link. Unfortunately, the definitions aren't entirely identical. The differences I've spotted so far:

/* From platforms/android-18/arch-x86/usr/include/asm/termbits.h */

#define NCCS 19 /* compared to 32 in linux.rs */

struct termios {
  tcflag_t c_iflag;
  tcflag_t c_oflag;
  tcflag_t c_cflag;
  tcflag_t c_lflag;
  cc_t c_line;
  cc_t c_cc[NCCS];
  /* missing c_ispeed and c_ospeed */
};

The smaller NCCS constant opens the possibility of accessing memory outside of the c_cc array, which is a memory safety error. I've only compared header files, so there's a chance they could be defined differently from a header file included elsewhere. However, the libc crate uses definitions that match what I'm seeing in the above header file.

NCCS: https://github.com/rust-lang/libc/blob/72b16d27cce111cbd4b9055d974d0ea70d152347/src/unix/notbsd/android/mod.rs#L678

struct termios: https://github.com/rust-lang/libc/blob/72b16d27cce111cbd4b9055d974d0ea70d152347/src/unix/notbsd/android/mod.rs#L78-L85

The other constants that I spot-checked match, so linux.rs is probably a good starting point for Android. Please copy linux.rs to android.rs and make any necessary changes, using the NDK header files as reference and the libc crate as a sanity check.

fornwall commented 5 years ago

The other constants that I spot-checked match, so linux.rs is probably a good starting point for Android. Please copy linux.rs to android.rs and make any necessary changes, using the NDK header files as reference and the libc crate as a sanity check.

This has been done in #19.

spk commented 5 years ago

Closing in favor of @fornwall PR #19