This review is from Crev, a distributed system for code reviews. To add your review, set up cargo-crev.

The current version of crossterm_input is 0.5.0.

0.4.1 (older version) Rating: Negative Thoroughness: Medium Understanding: Medium

by MaulingMonkey on 2019-09-25

Pros:

  • Handles console input

Cons:

  • Soundness issues
  • Not browser compatible
File Rating Notes
.github/CODEOWNERS +1
docs/CONTRIBUTING.md +1
src/input/input.rs +1
src/input/unix_input.rs -1 Parsing looks off, panicy internals
src/input/windows_input.rs -1 Unsound [#245], very strange keyboard handling.
src/sys/unix.rs 0 Mishandles read == 0?
src/input.rs +1
src/lib.rs +1
src/sys.rs +1
.cargo_vcs_info.json +1
.cargo-ok +1
.gitignore +1
travis.yml +1
Cargo.toml +1 MIT, winapi, libc, optional serde
Cargo.toml.orig +1 MIT, winapi, libc, optional serde
CHANGELOG.md +1
LICENSE +1 MIT
README.md +1 MIT
Other Rating Notes
unsafe -1 Unsound
fs +1 /dev/tty access, but that's expected
io +1 Sound... probably
docs +1
tests 0 Admittedly hard to unit test

src/input/unix_input.rs

Line What Notes
198 fn SyncReader::next -1, Disambiguating ESC like this seems super sketchy/brittle.
261 fn parse_event -1, This is more like what ESC parsing should look like...?
269 fn parse_event -1, \r\n -> Enter Enter? Seems wrong...
312 fn parse_csi -1, .unwrap Panic city
EOF

src/input/windows_input.rs

Line What Notes
42 static mut ORIG_MODE -1, More unsound access [[#2]]
47 WindowsInput::read_char +1, unsafe { ... } - willing to assume _getwche is sound.
110 fn WindowsInput::enable_mouse_mode -1, unsafe { ... } - unsound access of ORIG_MODE! [#245]
116 fn WindowsInput::disable_mouse_mode -1, unsafe { ... } - unsound access of ORIG_MODE! [#245]
225 fn read_single_event 0, unsafe { ... } - KeyEventRecord::from(*input.event.KeyEvent()) is probably sound/safe?
228 fn read_single_event 0, unsafe { ... } - MouseEvent::from(*input.event.MouseEvent()) is probably sound/safe?
249 fn read_input_events 0, unsafe { ... } - KeyEventRecord::from(*input.event.KeyEvent()) is probably sound/safe?
256 fn read_input_events 0, unsafe { ... } - MouseEvent::from(*input.event.MouseEvent()) is probably sound/safe?
291 fn parse_key_event_record 0, Several keys are dead, apparently
303 fn parse_key_event_record 0, Strange enumeration values for KeyEvent
345 fn parse_key_event_record -1, either VK_PRIOR | VK_NEXT can be sanely simplified a lot or something is super fucky.
354 fn parse_key_event_record -1, either VK_END | VK_HOME can be sanely simplified a lot or something is super fucky.
367 fn parse_key_event_record 0, unsafe { ... } assumes UnicodeChar is valid. Private fn only called on win32 results... technically unsound of winapi only populated AsciiChar, but that would be super dumb.
EOF

src/sys/unix.rs

Line What Notes
19 fn get_tty_fd +1, unsafe { ... } looks sound
45 fn read_char_raw 0, read == 0 can probably happen when pipe broken? generates extra ' '?
EOF

Crates in the crates.io registry are tarball snapshots uploaded by crates' publishers. The registry is not using crates' git repositories. There is absolutely no guarantee that the repository URL declared by the crate belongs to the crate, or that the code in the repository is the code inside the published tarball. To review the actual code of the crate, it's best to use cargo crev open crossterm_input. Alternatively, you can download the tarball of crossterm_input v0.5.0 or view the source online.