Lib.rs

› jni-sys › Reviews

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

The current version of jni-sys is 0.4.0.

0.3.0 (older version) Rating: Positive Thoroughness: Medium Understanding: High

by MaulingMonkey on 2019-07-30

Good solid FFI crate. Manual generation (I see comments!) concerns me, but upon review it looks to have been done correctly.

Verified all structs and FFI signatures against android JNI.h, with the exception of double checking that everything is marked JNICALL on windows JNI instead of just most of it. Android JNI lacks parameter names, so I didn't sanity check those either. Verified against %LOCALAPPDATA%\Android\Sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include\jni.h

Detail

File Rating Notes
src/lib.rs +1 Lots of unsafe... but necessary.
.gitignore +1
.travis.yml +1
appveyor.yml +1
Cargo.toml +1
Cargo.toml.orig +1
README.md +1
Other Rating Notes
unsafe +1 Lots of it... but all necessary, and after careful review, all appears correct.
fs +1 None
io +1 None
docs -1 Nonexistant, but of low importance (other primary references are available)
tests 0 Not in crate

src/lib.rs

Line Rating Notes
7 +1 Looks sufficiently correct to me - VS defines this as *mut c_char, but *mut c_void is close enough IMO.
10 +1 all verified
20 0 native version uses more newtypes, but without inheritence maybe this is sane in Rust.
39 +1 verified
51 +1 sure
57 +1
64 -1 DANGER - rust enum for C enum is begging for unsound if returned from FFI
71 +1 JNI constants here all look good
93 0 JNINativeMethod - C version uses const char*, but the muts here aren't a big deal... I
99 +1
105 +1 JNINativeInterface is bloody huge, but all methods appear to match in name, position, and signature...
115 ...with the caveat that there's a lot of "system" convention when JNICALL isn't used throughout the android jni.h. However, it is used in openjdk so that's probably fine?
116 I'm also assuming Option<unsafe extern "system" fn(...) -> ...> is FFI compatible with C function pointers, which might be a bad assumption.
157 Native version isn't marked no return but I bet it is.
175 Also, varargs functions degrade to "C" mode per https://stackoverflow.com/a/3615407
1411 Reviewed all the way to here!
1413 +1 Sure
1421 +1 JNIEnv_ - Lack of impl with forwarding fns like jni.h has is vageuly annoying.
1425 +1 Sure
1433 +1 JavaVMOption - Again, mut differences, but otherwise OK. Sure.
1438 +1 Sure
1446 +1 JavaVMInitArgs - LGTM
1453 +1 Sure
1461 +1 JavaVMAttachArgs - Again, mut differences, but otherwise OK. Sure.
1467 +1 Sure
1475 +1 JNIInvokeInterface - LGTM
1501 +1 Sure
1507 +1 LHTM

This review is from cargo-vet. To add your review, set up cargo-vet and submit your URL to its registry.

The current version of jni-sys is 0.4.0.

cargo-vet does not verify reviewers' identity. You have to fully trust the source the audits are from.

unknown

May have been packaged automatically without a review

Lib.rs has been able to verify that all files in the crate's tarball are in the crate's repository with a git tag matching the version. Please note that this check is still in beta, and absence of this confirmation does not mean that the files don't match.

Crates in the crates.io registry are tarball snapshots uploaded by crates' publishers. The registry is not using crates' git repositories, so there is a possibility that published crates have a misleading repository URL, or contain different code from the code in the repository.

To review the actual code of the crate, it's best to use cargo crev open jni-sys. Alternatively, you can download the tarball of jni-sys v0.4.0 or view the source online.