https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

https://64.github.io/actix/https://github.com/actix/actix-web/pull/968#issuecomment-509894555

Pros:

• Solid and awesome

Cons:

• unsafe in macro prevents use with #![forbid(unsafe_code)]
• Bumps MSRV on patch versions (but so far only for damn good reasons)

Full Audit

Pros:

• Solid and awesome

Cons:

• unsafe in macro prevents use with #![forbid(unsafe_code)]
• Bumps MSRV on patch versions (but so far only for damn good reasons)

Full Audit

Pros:

• Solid and awesome

Cons:

• unsafe in macro prevents use with #![forbid(unsafe_code)]
• Bumps MSRV on patch versions (but so far only for damn good reasons)

Full Audit

Pros:

• Solid and awesome

Cons:

• unsafe in macro prevents use with #![forbid(unsafe_code)]
• Bumps MSRV on patch versions (but so far only for damn good reasons)
• core::mem::uninitialized until 1.0.4 is UB

Full Audit

Pros:

• Solid and awesome

Cons:

• unsafe in macro prevents use with #![forbid(unsafe_code)]
• Bumps MSRV on patch versions (but so far only for damn good reasons)
• core::mem::uninitialized until 1.0.4 is UB

Full Audit

Pros:

• Solid and awesome

Cons:

• unsafe in macro prevents use with #![forbid(unsafe_code)]
• Bumps MSRV on patch versions (but so far only for damn good reasons)
• core::mem::uninitialized until 1.0.4 is UB

Full Audit

Pros:

• Solid and awesome

Cons:

• unsafe in macro prevents use with #![forbid(unsafe_code)]
• Bumps MSRV on patch versions (but so far only for damn good reasons)
• core::mem::uninitialized until 1.0.4 is UB

Full Audit

Pros:

• Solid and awesome

Cons:

• unsafe in macro prevents use with #![forbid(unsafe_code)]
• Bumps MSRV on patch versions (but so far only for damn good reasons)
• core::mem::uninitialized until 1.0.4 is UB

Full Audit

Stack/value variable length arrays without heap fallback.

Pros:

• Maybe sound?
• Better than what you'll write.

Cons:

• History of unsoundness (0.4.10 and earlier)
• Disturbing amounts of unsafe

This version switched some slices possibly containing uninit (UB!) to use
pointers instead. This makes encode_utf8 unsafe, sadly.

Stack/value variable length arrays without heap fallback.

Pros:

• Maybe sound?
• Better than what you'll write.

Cons:

• History of unsoundness (0.4.10 and earlier)
• Disturbing amounts of unsafe

Probably sound as of 0.4.11 on Rust 1.36.0+?
Uses a disturbing amount of unsafe, but aside from uninitialized! use in ArrayVec 1.35 and earlier, it all at least appears to be correct after a careful reading.
Unlike smallvec, this doesn't fall back on the heap.
Better than whatever you'll write rolling your own, at least.

0.4.7 -> 0.4.8: IntoIter implemented Clone, unconcerning
0.4.8 -> 0.4.9: ArrayString initialized to 0, ArrayVec uses nightly MaybeUninit. Unfortunately stable still uses uninitialized!() so this is still negative.
0.4.9 -> 0.4.10: #[repr(C)], -Clone for MaybeUninit. Apparently I missed more possible unsoundness. Unsafe is hard.
0.4.10 -> 0.4.11: ArrayVec should now also be sound in Rust 1.36.0+, probably, maybe.

Prefer 0.4.11 which at least starts using MaybeUninit instead of uninitialized!(), which is fundamentally unsound.
Uses a disturbing amount of unsafe, but aside from uninitialized, it all at least appears to be correct after a careful reading.
Unlike smallvec, this doesn't fall back on the heap.
Better than whatever you'll write rolling your own, at least.

0.4.7 -> 0.4.8: IntoIter implemented Clone, unconcerning
0.4.8 -> 0.4.9: ArrayString initialized to 0, ArrayVec uses nightly MaybeUninit. Unfortunately stable still uses uninitialized!() so this is still negative.
0.4.9 -> 0.4.10: #[repr(C)], -Clone for MaybeUninit. Apparently I missed more possible unsoundness. Unsafe is hard.

Prefer 0.4.11 which at least starts using MaybeUninit instead of uninitialized!(), which is fundamentally unsound.
Uses a disturbing amount of unsafe, but aside from uninitialized, it all at least appears to be correct after a careful reading.
Unlike smallvec, this doesn't fall back on the heap.
Better than whatever you'll write rolling your own, at least.

0.4.7 -> 0.4.8: IntoIter implemented Clone, unconcerning
0.4.8 -> 0.4.9: ArrayString initialized to 0, ArrayVec uses nightly MaybeUninit. Unfortunately stable still uses uninitialized!() so this is still negative.

Prefer 0.4.11 which at least starts using MaybeUninit instead of uninitialized!(), which is fundamentally unsound.
Uses a disturbing amount of unsafe, but aside from uninitialized, it all at least appears to be correct after a careful reading.
Unlike smallvec, this doesn't fall back on the heap.
Better than whatever you'll write rolling your own, at least.

See 0.4.7 for base review. Diffed 0.4.7 -> 0.4.8, no concerning changes.

Prefer 0.4.11 which at least starts using MaybeUninit instead of uninitialized!(), which is fundamentally unsound.
Uses a disturbing amount of unsafe, but aside from uninitialized, it all at least appears to be correct after a careful reading.
Unlike smallvec, this doesn't fall back on the heap.
Better than whatever you'll write rolling your own, at least.

Detail

src/array_string.rs

OK

src/array.rs

UNSOUND: https://github.com/tomprogrammer/rust-ascii/issues/64 (I didn't cath this one)
unsound? https://github.com/tomprogrammer/rust-ascii/issues/65

Lots of repeated unsafe.
No debug_assert! s for uncheck codepaths.
Unsound test code assumes gen_range meets it's safe API contract.
Not fully reviewed.

Detail

src/ascii_char.rs

src/ascii_str.rs

UNSOUND: https://github.com/tomprogrammer/rust-ascii/issues/64 (I didn't catch this one)
unsound? https://github.com/tomprogrammer/rust-ascii/issues/65

Lots of repeated unsafe.
No debug_assert! s for uncheck codepaths.
Unsound test code assumes gen_range meets it's safe API contract.
Not fully reviewed.

Detail

src/ascii_char.rs

src/ascii_str.rs

Not suitable for UGC - memory exhaustion and panic concerns.
https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/ase.md#0.1.3

Not suitable for UGC - memory exhaustion and panic concerns.
https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/ase.md#0.1.1

Not suitable for UGC - memory exhaustion and panic concerns.
https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/ase.md#0.1.0

[Full Audit]
Parses JSON aseprite exports

Pros:

• Looks fairly exhaustive, self contained
• No direct disk I/O

Cons:

• Exahustively matchable structs make most version bumps technically breaking changes
• Requires non-default export options (fixed in an unmerged PR)
• Enum deserialization is brittle when it comes to new options
• Limited maintainence

[Full Audit]
Parses JSON aseprite exports

Pros:

• Looks fairly exhaustive, self contained
• No direct disk I/O

Cons:

• Exahustively matchable structs make most version bumps technically breaking changes
• Requires non-default export options (fixed in an unmerged PR)
• Enum deserialization is brittle when it comes to new options
• Limited maintainence

Private fields make this unusable until 0.1.2

Private fields make this unusable until 0.1.2

LGTM, starts using RUSTFLAGS

0.1.6: LGTM
0.1.5: No unsafe code, minor safe-looking file I/O

No unsafe code, minor safe-looking file I/O

Offset read/write with &self

Pros:

• No runtime dependencies
• read_at(&self, ...)

Cons:

• BufOffsetReader is a bit kludgy - u64 as usize casts should probably Err instead
• Implementing traits directly on File makes for inconsistent seek behavior when used by reference

Full Audit

Offset read/write with &self

Pros:

• No runtime dependencies
• read_at(&self, ...)

Cons:

• BufOffsetReader is a bit kludgy - u64 as usize casts should probably Err instead
• Implementing traits directly on File makes for inconsistent seek behavior when used by reference

Full Audit

Offset read/write with &self

Pros:

• No runtime dependencies
• read_at(&self, ...)

Cons:

• BufOffsetReader is a bit kludgy - u64 as usize casts should probably Err instead
• Implementing traits directly on File makes for inconsistent seek behavior when used by reference

Full Audit

Offset read/write with &self

Pros:

• No runtime dependencies
• read_at(&self, ...)

Cons:

• BufOffsetReader is a bit kludgy - u64 as usize casts should probably Err instead
• Implementing traits directly on File makes for inconsistent seek behavior when used by reference

Full Audit

Offset read/write with &self

Pros:

• No runtime dependencies
• read_at(&self, ...)

Cons:

• BufOffsetReader is a bit kludgy - u64 as usize casts should probably Err instead
• Implementing traits directly on File makes for inconsistent seek behavior when used by reference

Full Audit

Offset read/write with &self

Pros:

• No runtime dependencies
• read_at(&self, ...)

Cons:

• BufOffsetReader is a bit kludgy - u64 as usize casts should probably Err instead
• Implementing traits directly on File makes for inconsistent seek behavior when used by reference

Full Audit

Offset read/write with &self

Pros:

• No runtime dependencies
• read_at(&self, ...)

Cons:

• BufOffsetReader is a bit kludgy - u64 as usize casts should probably Err instead
• Implementing traits directly on File makes for inconsistent seek behavior when used by reference

Full Audit

I wrote it, minimal 'unsafe' for FFI, well tested.

See Full Audit

• MSRV policy!
• Basic swizzling/endian stuff
• You didn't have to write it

• Excessive and distributed unsafe in serialization related code is hard to audit and makes me nervous
• History of unsoundness. Most crate versions have alignment related bugs, and 0.2.x had overflow issues which regressed in 0.3.x.
• Very limited functionality

Part of my byteorder audit

Part of my byteorder audit

Part of my byteorder audit

Part of my byteorder audit

https://github.com/rust-lang/cargo - I blindly trust rust

A mixed bag.
On the one hand, the core tool found in src seems fine, solid fundamentals.
On the other hand, the glue code is full chock full of unsafe, some of it unsound.
Worse, the FFI includes things that varies from android SDK to SDK from the loops of things.

injected-glue\ffi.rs: -1
Should be replaced with bindgen generated header? Also, changes between SDKs, so really really really should not be simply hardcoded...!
Ref: https://android.googlesource.com/platform/development/+/4948c163663ecc343c97e4c2a2139234f1d3273f/ndk/sources/android/native_app_glue/android_native_app_glue.h
Ref: https://chromium.googlesource.com/android_tools/+/7fc902d157a9aed7a2b68adc9c69181b3a43cd58/ndk/sources/android/native_app_glue/android_native_app_glue.h
45 LOOPER_ID_INPUT: Wrong? (should be 2?: https://chromium.googlesource.com/android_tools/+/7fc902d157a9aed7a2b68adc9c69181b3a43cd58/ndk/sources/android/native_app_glue/android_native_app_glue.h#204)
LOOPER_ID_EVENT? Different name? https://android.googlesource.com/platform/development/+/4948c163663ecc343c97e4c2a2139234f1d3273f/ndk/sources/android/native_app_glue/android_native_app_glue.h
46 LOOPER_ID_USER: Wrong? (should be 3?: https://chromium.googlesource.com/android_tools/+/7fc902d157a9aed7a2b68adc9c69181b3a43cd58/ndk/sources/android/native_app_glue/android_native_app_glue.h#209)
65+ Skimmed only

injected-glue\lib.rs: -1
92 static mut ANDROID_APP should be UnsafeCell
164 static mut G_MAINTHREAD_BOXED should be UnsafeCell, probable undefined behavior / race conditions
172 scary transmutes
182 Unsound when combined with
191 'safe' access
196 android_main2 is UNSOUND - dereferencing pointers, setting globals, etc. without being marked unsafe.
197+ Skimmed only
340: giant unsafe block
347: uninitialized

src\ops\build.rs: +1
75 Medium: I suspect this won't work on windows as it doesn't use .cmd scripts for the compiler/linker?
131 Medium: injected_glue_lib cmd.exec()?; followed by cmd.exec_with_output()?; (134)- does the glue get double compiled for no good reason?
218 Medium: unreachable! abused where panic! probably should be?
423 Minor: Hardcodes a lot of assumptions. Good foundation though.

src\ops\install.rs: +1
Medium: No way to specify device
src\ops\mod.rs: +1
src\ops\run.rs: +1
Medium: No way to specify device

src\config.rs: +1
Minor: Could use more OsString
Minor: Non-optional opengles_version_*
src\main.rs: +1

Cargo.toml: +1
Cargo.toml.orig: +1
glue_obj.rs: Sketchy looking casts and main handling... but might be right
linker.rs: Strange parse_arguments loop style... but safe, looks sane.

0.4.16: Trivial version bumps.
0.4.15: Trivial version bumps.
0.4.14: Trivial version bumps, mass reformatting.
0.4.13: Trivial version bumps.
0.4.12: Trivial version bumps.
0.4.11: Nice and solid looking code. 100% safe code.

Reviewed:
src\cli.rs: +1
src\main.rs: +1
151 run_lldb
Nonterminating loops are problematic, but I think the side effects here should make this work OK?
(see https://github.com/rust-lang/rust/issues/28728 )
Cargo.toml: +1
Cargo.toml.orig: +1
13 readme path references unpackaged readme

TIL:
Using traits to extend clap::App so flags can be reused per-subcommand is interesting.

0.4.15: Trivial version bumps.
0.4.14: Trivial version bumps, mass reformatting.
0.4.13: Trivial version bumps.
0.4.12: Trivial version bumps.
0.4.11: Nice and solid looking code. 100% safe code.

Reviewed:
src\cli.rs: +1
src\main.rs: +1
151 run_lldb
Nonterminating loops are problematic, but I think the side effects here should make this work OK?
(see https://github.com/rust-lang/rust/issues/28728 )
Cargo.toml: +1
Cargo.toml.orig: +1
13 readme path references unpackaged readme

TIL:
Using traits to extend clap::App so flags can be reused per-subcommand is interesting.

0.4.14: Trivial version bumps, mass reformatting.
0.4.13: Trivial version bumps.
0.4.12: Trivial version bumps.
0.4.11: Nice and solid looking code. 100% safe code.

Reviewed:
src\cli.rs: +1
src\main.rs: +1
151 run_lldb
Nonterminating loops are problematic, but I think the side effects here should make this work OK?
(see https://github.com/rust-lang/rust/issues/28728 )
Cargo.toml: +1
Cargo.toml.orig: +1
13 readme path references unpackaged readme

TIL:
Using traits to extend clap::App so flags can be reused per-subcommand is interesting.

0.4.13: Trivial version bumps.
0.4.12: Trivial version bumps.
0.4.11: Nice and solid looking code. 100% safe code.

Reviewed:
src\cli.rs: +1
src\main.rs: +1
151 run_lldb
Nonterminating loops are problematic, but I think the side effects here should make this work OK?
(see https://github.com/rust-lang/rust/issues/28728 )
Cargo.toml: +1
Cargo.toml.orig: +1
13 readme path references unpackaged readme

TIL:
Using traits to extend clap::App so flags can be reused per-subcommand is interesting.

0.4.12: Trivial version bumps.
0.4.11: Nice and solid looking code. 100% safe code.

Reviewed:
src\cli.rs: +1
src\main.rs: +1
151 run_lldb
Nonterminating loops are problematic, but I think the side effects here should make this work OK?
(see https://github.com/rust-lang/rust/issues/28728 )
Cargo.toml: +1
Cargo.toml.orig: +1
13 readme path references unpackaged readme

TIL:
Using traits to extend clap::App so flags can be reused per-subcommand is interesting.

Nice and solid looking code. 100% safe code.

Reviewed:
src\cli.rs: +1
src\main.rs: +1
151 run_lldb
Nonterminating loops are problematic, but I think the side effects here should make this work OK?
(see https://github.com/rust-lang/rust/issues/28728 )
Cargo.toml: +1
Cargo.toml.orig: +1
13 readme path references unpackaged readme

TIL:
Using traits to extend clap::App so flags can be reused per-subcommand is interesting.

Add/remove/update Cargo.toml dependencies from the command line.

Pros:

• Safe, probably works

Cons:

• Lots of code
• Lots of dependencies, some of which I haven't fully audited.
• License ambiguity (is this MIT or Apache-2.0/MIT licensed?)

0.4.0

Add/remove/update Cargo.toml dependencies from the command line.

Pros:

• Safe, probably works

Cons:

• Lots of code
• Lots of dependencies, some of which I haven't fully audited.
• No Cargo.lock so not installable with --frozen
• License ambiguity (is this MIT or Apache-2.0/MIT licensed?)

0.3.3

TIL

• crates.io API string format
• Refresher on format! placeholders

format!(
    "{host}/api/v1/crates/{crate_name}",
    host = REGISTRY_HOST,
    crate_name = crate_name
);

Pretty trivial, not even sure if it'll work right on windows to be worth using.

Parse cargo metadata and cargo build --message-format=json output.

Pros:

• Way better than parsing it yourself
• Safe code

Cons:

• If you're feeling particularly paranoid, cargo metadata could be passed bad
  feature names (see 0.8.2 review for details)

0.9.1

Parse cargo metadata and cargo build --message-format=json output.

0.9.0

0.8.2

src/lib.rs

Parse cargo metadata and cargo build --message-format=json output.

0.8.2

src/lib.rs

Macro black magic... I trust the author and what I do understand looks good.

Macro black magic... I trust the author and what I do understand looks good.

Interesting idea. Lots of unsafe! Supposedly auto-generated.
I have at least read the code enough (including explicitly looking at each use of unsafe)
to believe there are no obvious funny business nor proper soundness issues. That said:

Concerns (rating: neutral):
Lots of concerning use of unsafe { ... } + core::mem::uninitialized()... but only in #[test] code.
#[allow(improper_ctypes)] for extern "C" block. Sounds like UB-bait.
Punts all safety concerns to the user.

Concerns (thoroughness: low):
I have not properly vetted fn/struct/type signatures against a proper reference.

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/codepage-437.md#0.1.0

Pros:

• Cross platform

Cons:

• Soundness issues
• Data races

src/sys/winapi.rs

Pros:

• Cross platform

Cons:

• Soundness issues
• Data races

src/sys/winapi.rs

Pros:

• Handles console input

Cons:

• Soundness issues
• Not browser compatible

src/input/unix_input.rs

src/input/windows_input.rs

src/sys/unix.rs

Pros:

• Abstracts platform specific bits
• This part is safe code

0.3.1

Pros:

• Styling!

Cons:

• Poor win7 support
• Unsound

src/winapi_color.rs

0.4.16: Trivial version bumps.
0.4.15: Trivial version bumps.
0.4.14: Trivial version bumps, style changes.
0.4.13: Trivial version bumps.
0.4.12: Trivial version bumps.
0.4.11
Some of the build/path stuff seems a little off... but might be correct?
All safe code, no security problems.

Reviewed:

src\build_env.rs    +1
src\build.rs        +1
src\lib.rs          0
    102 Is this really correct for specifying the *host* environment?
    105 ..
src\utils.rs        0
    14  Isn't this generating /../../../ ?  Doesn't seem right...

Cargo.toml          +1
Cargo.toml.orig     +1

0.4.15: Trivial version bumps.
0.4.14: Trivial version bumps, style changes.
0.4.13: Trivial version bumps.
0.4.12: Trivial version bumps.
0.4.11
Some of the build/path stuff seems a little off... but might be correct?
All safe code, no security problems.

Reviewed:

src\build_env.rs    +1
src\build.rs        +1
src\lib.rs          0
    102 Is this really correct for specifying the *host* environment?
    105 ..
src\utils.rs        0
    14  Isn't this generating /../../../ ?  Doesn't seem right...

Cargo.toml          +1
Cargo.toml.orig     +1

0.4.14: Trivial version bumps, style changes.
0.4.13: Trivial version bumps.
0.4.12: Trivial version bumps.
0.4.11
Some of the build/path stuff seems a little off... but might be correct?
All safe code, no security problems.

Reviewed:

src\build_env.rs    +1
src\build.rs        +1
src\lib.rs          0
    102 Is this really correct for specifying the *host* environment?
    105 ..
src\utils.rs        0
    14  Isn't this generating /../../../ ?  Doesn't seem right...

Cargo.toml          +1
Cargo.toml.orig     +1

0.4.13: Trivial version bumps.
0.4.12: Trivial version bumps.
0.4.11
Some of the build/path stuff seems a little off... but might be correct?
All safe code, no security problems.

Reviewed:

src\build_env.rs    +1
src\build.rs        +1
src\lib.rs          0
    102 Is this really correct for specifying the *host* environment?
    105 ..
src\utils.rs        0
    14  Isn't this generating /../../../ ?  Doesn't seem right...

Cargo.toml          +1
Cargo.toml.orig     +1

0.4.12: Trivial version bumps.
0.4.11
Some of the build/path stuff seems a little off... but might be correct?
All safe code, no security problems.

Reviewed:

src\build_env.rs    +1
src\build.rs        +1
src\lib.rs          0
    102 Is this really correct for specifying the *host* environment?
    105 ..
src\utils.rs        0
    14  Isn't this generating /../../../ ?  Doesn't seem right...

Cargo.toml          +1
Cargo.toml.orig     +1

Some of the build/path stuff seems a little off... but might be correct?
All safe code, no security problems.

Reviewed:

src\build_env.rs    +1
src\build.rs        +1
src\lib.rs          0
    102 Is this really correct for specifying the *host* environment?
    105 ..
src\utils.rs        0
    14  Isn't this generating /../../../ ?  Doesn't seem right...

Cargo.toml          +1
Cargo.toml.orig     +1

0.4.16: Trivial version bumps, thread -> std::thread
0.4.15: Trivial version bumps
0.4.14: Trivial version bumps, lots of pointless style changes, a few warning fixes (missing dyns etc.)
0.4.13: Trivial version bumps
0.4.12: Trivial version bumps
0.4.11
thoroughness: low - mostly due to iOS FFI, shell stuff not being super thorough, and review fatigue causing my eyes to glaze over in places.
understanding: low - lots of shell/path stuff that I don't know well enough to verify
rating: neutral - see concerns bellow.

Concerns

• Looks like lots of stuff might not work on windows... although there is windows-specific code, so maybe?
• Lots of unimplemented!()
• iOS support is chock full of unsafe { ... } for FFI. I haven't verified the FFI signatures.
• iOS support also uses unsafe { ... } for several objective C casts. Needs some sanity checked utility functions.
• iOS support has some potential UB during panics due to unwinding over FFI boundaries.
• Implements some code signing stuff for iOS. Necessary, and just your local certs, but I haven't fully reasoned through what security impacts if any that has.
• Few unit tests visible in the crate itself (maybe they're separate and unpackaged?)
• Sandboxing concerns
  • Remotes into other devices, including over ssh.
  • Frequent use of shell commands could lead to build server RCEs given malicious project metadata
  • Malicious projects will just use a build.rs file though, front door is open so to speak.

Details

src\android\device.rs +1
14 Odd place to install...
src\android\mod.rs +1
155 Could also check android studio SDK install path

src\host\device.rs +1
src\host\mod.rs +1
src\host\platform.rs +1

src\ios\device.rs 0
50 Not sure if the underlying iOS APIs are thread safe, but this seems acceptable.
277 unsafe { ... } for FFI. Looks generally safe except for scary mem::transmute(kCFBooleanTrue), but even that may be right.
303 unsafe { ... } for FFI. Looks safe to me.
367 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
419 unsafe { ... } for FFI. Looks safe to me.
440 unsafe { ... } for FFI. Looks safe to me.
454 unsafe { ... } for FFI. Looks safe to me.
487 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
519 This thread just eats errors.
683 unsafe { ... } for FFI. Transmute... probably safe.
690 unsafe { ... } for FFI. Looks safe to me.
691 unsafe { ... } for FFI. Scary Core Foundation related transmutes... probably OK, but some utilities to sanity check these conversions in debug would be nice.
696 unsafe { ... } for FFI. Another scary-but-probably-safe transmute.
src\ios\helpers.py +1
src\ios\mobiledevice_sys.rs 0
FFI, not perfectly verified since I don't have an OS X machine to check the headers out on.
Ref: https://github.com/PanayotCankov/device.io/blob/master/idb/MobileDevice.h
Various minor const differences, a few functions missing in ref, a few likely improved definitions in places.
33 am_device_notification_callback_info has "extra" field vs reference, "subscription". iOS internal struct?
src\ios\mod.rs 0
43 unsafe { ... } for FFI. Looks safe to me.
55 technically unsound inner fn (uses ptrs)
61 Scary looking as hell, but I think this is just going Box (40) -> void* (48) -> Box (here).
62 UNDEFINED BEHAVIOR: Possible panic unwind through FFI, technically an hazard.
I would've missed this edge case but for the comment, so I'm not docking points.
Unlikely to cause severe problems, but would be worth fixing.
src\ios\platform.rs +1
src\ios\xcode.rs +1
25 Dead code not reviewed
91 I have not thoroughly audited this code signing stuff, but looks OK.
199 com.zoy.kali.Dinghy? A bit hardcoded...

src\platform\mod.rs +1
src\platform\regular_platform.rs +1
44 Since when does "regular" mean "*nix" - might not work on windows.

src\script\device.rs +1
src\script\mod.rs +1

src\ssh\device.rs 0
src\ssh\mod.rs +1

src\compiler.rs +1
475 Isn't this unbanned?
src\config.rs +1
src\device.rs +1
src\errors.rs +1
src\lib.rs +1
87 random sleep? why?
src\overlay.rs +1
src\project.rs +1
src\toolchain.rs +1
src\utils.rs +1

build.rs +1
Cargo.toml +1
Cargo.toml.orig +1

TIL

Neat loop pattern:

for (a,     b,      c) in &[
    ("a",   "b",    "c"),
    ("aa",  "bb",   "cc"),
] {
    ...
}

Sysroot paths: ndk/toolchains/llvm/prebuilt/sysroot/usr/lib/{binutils_cpu}-linux-{abi_kind}"

0.4.15: Trivial version bumps
0.4.14: Trivial version bumps, lots of pointless style changes, a few warning fixes (missing dyns etc.)
0.4.13: Trivial version bumps
0.4.12: Trivial version bumps
0.4.11
thoroughness: low - mostly due to iOS FFI, shell stuff not being super thorough, and review fatigue causing my eyes to glaze over in places.
understanding: low - lots of shell/path stuff that I don't know well enough to verify
rating: neutral - see concerns bellow.

Concerns

• Looks like lots of stuff might not work on windows... although there is windows-specific code, so maybe?
• Lots of unimplemented!()
• iOS support is chock full of unsafe { ... } for FFI. I haven't verified the FFI signatures.
• iOS support also uses unsafe { ... } for several objective C casts. Needs some sanity checked utility functions.
• iOS support has some potential UB during panics due to unwinding over FFI boundaries.
• Implements some code signing stuff for iOS. Necessary, and just your local certs, but I haven't fully reasoned through what security impacts if any that has.
• Few unit tests visible in the crate itself (maybe they're separate and unpackaged?)
• Sandboxing concerns
  • Remotes into other devices, including over ssh.
  • Frequent use of shell commands could lead to build server RCEs given malicious project metadata
  • Malicious projects will just use a build.rs file though, front door is open so to speak.

Details

src\android\device.rs +1
14 Odd place to install...
src\android\mod.rs +1
155 Could also check android studio SDK install path

src\host\device.rs +1
src\host\mod.rs +1
src\host\platform.rs +1

src\ios\device.rs 0
50 Not sure if the underlying iOS APIs are thread safe, but this seems acceptable.
277 unsafe { ... } for FFI. Looks generally safe except for scary mem::transmute(kCFBooleanTrue), but even that may be right.
303 unsafe { ... } for FFI. Looks safe to me.
367 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
419 unsafe { ... } for FFI. Looks safe to me.
440 unsafe { ... } for FFI. Looks safe to me.
454 unsafe { ... } for FFI. Looks safe to me.
487 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
519 This thread just eats errors.
683 unsafe { ... } for FFI. Transmute... probably safe.
690 unsafe { ... } for FFI. Looks safe to me.
691 unsafe { ... } for FFI. Scary Core Foundation related transmutes... probably OK, but some utilities to sanity check these conversions in debug would be nice.
696 unsafe { ... } for FFI. Another scary-but-probably-safe transmute.
src\ios\helpers.py +1
src\ios\mobiledevice_sys.rs 0
FFI, not perfectly verified since I don't have an OS X machine to check the headers out on.
Ref: https://github.com/PanayotCankov/device.io/blob/master/idb/MobileDevice.h
Various minor const differences, a few functions missing in ref, a few likely improved definitions in places.
33 am_device_notification_callback_info has "extra" field vs reference, "subscription". iOS internal struct?
src\ios\mod.rs 0
43 unsafe { ... } for FFI. Looks safe to me.
55 technically unsound inner fn (uses ptrs)
61 Scary looking as hell, but I think this is just going Box (40) -> void* (48) -> Box (here).
62 UNDEFINED BEHAVIOR: Possible panic unwind through FFI, technically an hazard.
I would've missed this edge case but for the comment, so I'm not docking points.
Unlikely to cause severe problems, but would be worth fixing.
src\ios\platform.rs +1
src\ios\xcode.rs +1
25 Dead code not reviewed
91 I have not thoroughly audited this code signing stuff, but looks OK.
199 com.zoy.kali.Dinghy? A bit hardcoded...

src\platform\mod.rs +1
src\platform\regular_platform.rs +1
44 Since when does "regular" mean "*nix" - might not work on windows.

src\script\device.rs +1
src\script\mod.rs +1

src\ssh\device.rs 0
src\ssh\mod.rs +1

src\compiler.rs +1
475 Isn't this unbanned?
src\config.rs +1
src\device.rs +1
src\errors.rs +1
src\lib.rs +1
87 random sleep? why?
src\overlay.rs +1
src\project.rs +1
src\toolchain.rs +1
src\utils.rs +1

build.rs +1
Cargo.toml +1
Cargo.toml.orig +1

TIL

Neat loop pattern:

for (a,     b,      c) in &[
    ("a",   "b",    "c"),
    ("aa",  "bb",   "cc"),
] {
    ...
}

Sysroot paths: ndk/toolchains/llvm/prebuilt/sysroot/usr/lib/{binutils_cpu}-linux-{abi_kind}"

0.4.14: Trivial version bumps, lots of pointless style changes, a few warning fixes (missing dyns etc.)
0.4.13: Trivial version bumps
0.4.12: Trivial version bumps
0.4.11
thoroughness: low - mostly due to iOS FFI, shell stuff not being super thorough, and review fatigue causing my eyes to glaze over in places.
understanding: low - lots of shell/path stuff that I don't know well enough to verify
rating: neutral - see concerns bellow.

Concerns

• Looks like lots of stuff might not work on windows... although there is windows-specific code, so maybe?
• Lots of unimplemented!()
• iOS support is chock full of unsafe { ... } for FFI. I haven't verified the FFI signatures.
• iOS support also uses unsafe { ... } for several objective C casts. Needs some sanity checked utility functions.
• iOS support has some potential UB during panics due to unwinding over FFI boundaries.
• Implements some code signing stuff for iOS. Necessary, and just your local certs, but I haven't fully reasoned through what security impacts if any that has.
• Few unit tests visible in the crate itself (maybe they're separate and unpackaged?)
• Sandboxing concerns
  • Remotes into other devices, including over ssh.
  • Frequent use of shell commands could lead to build server RCEs given malicious project metadata
  • Malicious projects will just use a build.rs file though, front door is open so to speak.

Details

src\android\device.rs +1
14 Odd place to install...
src\android\mod.rs +1
155 Could also check android studio SDK install path

src\host\device.rs +1
src\host\mod.rs +1
src\host\platform.rs +1

src\ios\device.rs 0
50 Not sure if the underlying iOS APIs are thread safe, but this seems acceptable.
277 unsafe { ... } for FFI. Looks generally safe except for scary mem::transmute(kCFBooleanTrue), but even that may be right.
303 unsafe { ... } for FFI. Looks safe to me.
367 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
419 unsafe { ... } for FFI. Looks safe to me.
440 unsafe { ... } for FFI. Looks safe to me.
454 unsafe { ... } for FFI. Looks safe to me.
487 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
519 This thread just eats errors.
683 unsafe { ... } for FFI. Transmute... probably safe.
690 unsafe { ... } for FFI. Looks safe to me.
691 unsafe { ... } for FFI. Scary Core Foundation related transmutes... probably OK, but some utilities to sanity check these conversions in debug would be nice.
696 unsafe { ... } for FFI. Another scary-but-probably-safe transmute.
src\ios\helpers.py +1
src\ios\mobiledevice_sys.rs 0
FFI, not perfectly verified since I don't have an OS X machine to check the headers out on.
Ref: https://github.com/PanayotCankov/device.io/blob/master/idb/MobileDevice.h
Various minor const differences, a few functions missing in ref, a few likely improved definitions in places.
33 am_device_notification_callback_info has "extra" field vs reference, "subscription". iOS internal struct?
src\ios\mod.rs 0
43 unsafe { ... } for FFI. Looks safe to me.
55 technically unsound inner fn (uses ptrs)
61 Scary looking as hell, but I think this is just going Box (40) -> void* (48) -> Box (here).
62 UNDEFINED BEHAVIOR: Possible panic unwind through FFI, technically an hazard.
I would've missed this edge case but for the comment, so I'm not docking points.
Unlikely to cause severe problems, but would be worth fixing.
src\ios\platform.rs +1
src\ios\xcode.rs +1
25 Dead code not reviewed
91 I have not thoroughly audited this code signing stuff, but looks OK.
199 com.zoy.kali.Dinghy? A bit hardcoded...

src\platform\mod.rs +1
src\platform\regular_platform.rs +1
44 Since when does "regular" mean "*nix" - might not work on windows.

src\script\device.rs +1
src\script\mod.rs +1

src\ssh\device.rs 0
src\ssh\mod.rs +1

src\compiler.rs +1
475 Isn't this unbanned?
src\config.rs +1
src\device.rs +1
src\errors.rs +1
src\lib.rs +1
87 random sleep? why?
src\overlay.rs +1
src\project.rs +1
src\toolchain.rs +1
src\utils.rs +1

build.rs +1
Cargo.toml +1
Cargo.toml.orig +1

TIL

Neat loop pattern:

for (a,     b,      c) in &[
    ("a",   "b",    "c"),
    ("aa",  "bb",   "cc"),
] {
    ...
}

Sysroot paths: ndk/toolchains/llvm/prebuilt/sysroot/usr/lib/{binutils_cpu}-linux-{abi_kind}"

0.4.13: Trivial version bumps
0.4.12: Trivial version bumps
0.4.11
thoroughness: low - mostly due to iOS FFI, shell stuff not being super thorough, and review fatigue causing my eyes to glaze over in places.
understanding: low - lots of shell/path stuff that I don't know well enough to verify
rating: neutral - see concerns bellow.

Concerns

• Looks like lots of stuff might not work on windows... although there is windows-specific code, so maybe?
• Lots of unimplemented!()
• iOS support is chock full of unsafe { ... } for FFI. I haven't verified the FFI signatures.
• iOS support also uses unsafe { ... } for several objective C casts. Needs some sanity checked utility functions.
• iOS support has some potential UB during panics due to unwinding over FFI boundaries.
• Implements some code signing stuff for iOS. Necessary, and just your local certs, but I haven't fully reasoned through what security impacts if any that has.
• Few unit tests visible in the crate itself (maybe they're separate and unpackaged?)
• Sandboxing concerns
  • Remotes into other devices, including over ssh.
  • Frequent use of shell commands could lead to build server RCEs given malicious project metadata
  • Malicious projects will just use a build.rs file though, front door is open so to speak.

Details

src\android\device.rs +1
14 Odd place to install...
src\android\mod.rs +1
155 Could also check android studio SDK install path

src\host\device.rs +1
src\host\mod.rs +1
src\host\platform.rs +1

src\ios\device.rs 0
50 Not sure if the underlying iOS APIs are thread safe, but this seems acceptable.
277 unsafe { ... } for FFI. Looks generally safe except for scary mem::transmute(kCFBooleanTrue), but even that may be right.
303 unsafe { ... } for FFI. Looks safe to me.
367 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
419 unsafe { ... } for FFI. Looks safe to me.
440 unsafe { ... } for FFI. Looks safe to me.
454 unsafe { ... } for FFI. Looks safe to me.
487 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
519 This thread just eats errors.
683 unsafe { ... } for FFI. Transmute... probably safe.
690 unsafe { ... } for FFI. Looks safe to me.
691 unsafe { ... } for FFI. Scary Core Foundation related transmutes... probably OK, but some utilities to sanity check these conversions in debug would be nice.
696 unsafe { ... } for FFI. Another scary-but-probably-safe transmute.
src\ios\helpers.py +1
src\ios\mobiledevice_sys.rs 0
FFI, not perfectly verified since I don't have an OS X machine to check the headers out on.
Ref: https://github.com/PanayotCankov/device.io/blob/master/idb/MobileDevice.h
Various minor const differences, a few functions missing in ref, a few likely improved definitions in places.
33 am_device_notification_callback_info has "extra" field vs reference, "subscription". iOS internal struct?
src\ios\mod.rs 0
43 unsafe { ... } for FFI. Looks safe to me.
55 technically unsound inner fn (uses ptrs)
61 Scary looking as hell, but I think this is just going Box (40) -> void* (48) -> Box (here).
62 UNDEFINED BEHAVIOR: Possible panic unwind through FFI, technically an hazard.
I would've missed this edge case but for the comment, so I'm not docking points.
Unlikely to cause severe problems, but would be worth fixing.
src\ios\platform.rs +1
src\ios\xcode.rs +1
25 Dead code not reviewed
91 I have not thoroughly audited this code signing stuff, but looks OK.
199 com.zoy.kali.Dinghy? A bit hardcoded...

src\platform\mod.rs +1
src\platform\regular_platform.rs +1
44 Since when does "regular" mean "*nix" - might not work on windows.

src\script\device.rs +1
src\script\mod.rs +1

src\ssh\device.rs 0
src\ssh\mod.rs +1

src\compiler.rs +1
475 Isn't this unbanned?
src\config.rs +1
src\device.rs +1
src\errors.rs +1
src\lib.rs +1
87 random sleep? why?
src\overlay.rs +1
src\project.rs +1
src\toolchain.rs +1
src\utils.rs +1

build.rs +1
Cargo.toml +1
Cargo.toml.orig +1

TIL

Neat loop pattern:

for (a,     b,      c) in &[
    ("a",   "b",    "c"),
    ("aa",  "bb",   "cc"),
] {
    ...
}

Sysroot paths: ndk/toolchains/llvm/prebuilt/sysroot/usr/lib/{binutils_cpu}-linux-{abi_kind}"

0.4.12: Trivial version bumps
0.4.11
thoroughness: low - mostly due to iOS FFI, shell stuff not being super thorough, and review fatigue causing my eyes to glaze over in places.
understanding: low - lots of shell/path stuff that I don't know well enough to verify
rating: neutral - see concerns bellow.

Concerns

• Looks like lots of stuff might not work on windows... although there is windows-specific code, so maybe?
• Lots of unimplemented!()
• iOS support is chock full of unsafe { ... } for FFI. I haven't verified the FFI signatures.
• iOS support also uses unsafe { ... } for several objective C casts. Needs some sanity checked utility functions.
• iOS support has some potential UB during panics due to unwinding over FFI boundaries.
• Implements some code signing stuff for iOS. Necessary, and just your local certs, but I haven't fully reasoned through what security impacts if any that has.
• Few unit tests visible in the crate itself (maybe they're separate and unpackaged?)
• Sandboxing concerns
  • Remotes into other devices, including over ssh.
  • Frequent use of shell commands could lead to build server RCEs given malicious project metadata
  • Malicious projects will just use a build.rs file though, front door is open so to speak.

Details

src\android\device.rs +1
14 Odd place to install...
src\android\mod.rs +1
155 Could also check android studio SDK install path

src\host\device.rs +1
src\host\mod.rs +1
src\host\platform.rs +1

src\ios\device.rs 0
50 Not sure if the underlying iOS APIs are thread safe, but this seems acceptable.
277 unsafe { ... } for FFI. Looks generally safe except for scary mem::transmute(kCFBooleanTrue), but even that may be right.
303 unsafe { ... } for FFI. Looks safe to me.
367 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
419 unsafe { ... } for FFI. Looks safe to me.
440 unsafe { ... } for FFI. Looks safe to me.
454 unsafe { ... } for FFI. Looks safe to me.
487 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
519 This thread just eats errors.
683 unsafe { ... } for FFI. Transmute... probably safe.
690 unsafe { ... } for FFI. Looks safe to me.
691 unsafe { ... } for FFI. Scary Core Foundation related transmutes... probably OK, but some utilities to sanity check these conversions in debug would be nice.
696 unsafe { ... } for FFI. Another scary-but-probably-safe transmute.
src\ios\helpers.py +1
src\ios\mobiledevice_sys.rs 0
FFI, not perfectly verified since I don't have an OS X machine to check the headers out on.
Ref: https://github.com/PanayotCankov/device.io/blob/master/idb/MobileDevice.h
Various minor const differences, a few functions missing in ref, a few likely improved definitions in places.
33 am_device_notification_callback_info has "extra" field vs reference, "subscription". iOS internal struct?
src\ios\mod.rs 0
43 unsafe { ... } for FFI. Looks safe to me.
55 technically unsound inner fn (uses ptrs)
61 Scary looking as hell, but I think this is just going Box (40) -> void* (48) -> Box (here).
62 UNDEFINED BEHAVIOR: Possible panic unwind through FFI, technically an hazard.
I would've missed this edge case but for the comment, so I'm not docking points.
Unlikely to cause severe problems, but would be worth fixing.
src\ios\platform.rs +1
src\ios\xcode.rs +1
25 Dead code not reviewed
91 I have not thoroughly audited this code signing stuff, but looks OK.
199 com.zoy.kali.Dinghy? A bit hardcoded...

src\platform\mod.rs +1
src\platform\regular_platform.rs +1
44 Since when does "regular" mean "*nix" - might not work on windows.

src\script\device.rs +1
src\script\mod.rs +1

src\ssh\device.rs 0
src\ssh\mod.rs +1

src\compiler.rs +1
475 Isn't this unbanned?
src\config.rs +1
src\device.rs +1
src\errors.rs +1
src\lib.rs +1
87 random sleep? why?
src\overlay.rs +1
src\project.rs +1
src\toolchain.rs +1
src\utils.rs +1

build.rs +1
Cargo.toml +1
Cargo.toml.orig +1

TIL

Neat loop pattern:

for (a,     b,      c) in &[
    ("a",   "b",    "c"),
    ("aa",  "bb",   "cc"),
] {
    ...
}

Sysroot paths: ndk/toolchains/llvm/prebuilt/sysroot/usr/lib/{binutils_cpu}-linux-{abi_kind}"

thoroughness: low - mostly due to iOS FFI, shell stuff not being super thorough, and review fatigue causing my eyes to glaze over in places.
understanding: low - lots of shell/path stuff that I don't know well enough to verify
rating: neutral - see concerns bellow.

Concerns

• Looks like lots of stuff might not work on windows... although there is windows-specific code, so maybe?
• Lots of unimplemented!()
• iOS support is chock full of unsafe { ... } for FFI. I haven't verified the FFI signatures.
• iOS support also uses unsafe { ... } for several objective C casts. Needs some sanity checked utility functions.
• iOS support has some potential UB during panics due to unwinding over FFI boundaries.
• Implements some code signing stuff for iOS. Necessary, and just your local certs, but I haven't fully reasoned through what security impacts if any that has.
• Few unit tests visible in the crate itself (maybe they're separate and unpackaged?)
• Sandboxing concerns
  • Remotes into other devices, including over ssh.
  • Frequent use of shell commands could lead to build server RCEs given malicious project metadata
  • Malicious projects will just use a build.rs file though, front door is open so to speak.

Details

src\android\device.rs +1
14 Odd place to install...
src\android\mod.rs +1
155 Could also check android studio SDK install path

src\host\device.rs +1
src\host\mod.rs +1
src\host\platform.rs +1

src\ios\device.rs 0
50 Not sure if the underlying iOS APIs are thread safe, but this seems acceptable.
277 unsafe { ... } for FFI. Looks generally safe except for scary mem::transmute(kCFBooleanTrue), but even that may be right.
303 unsafe { ... } for FFI. Looks safe to me.
367 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
419 unsafe { ... } for FFI. Looks safe to me.
440 unsafe { ... } for FFI. Looks safe to me.
454 unsafe { ... } for FFI. Looks safe to me.
487 unsafe { ... } for FFI. Looks safe to me, but needlessly large.
519 This thread just eats errors.
683 unsafe { ... } for FFI. Transmute... probably safe.
690 unsafe { ... } for FFI. Looks safe to me.
691 unsafe { ... } for FFI. Scary Core Foundation related transmutes... probably OK, but some utilities to sanity check these conversions in debug would be nice.
696 unsafe { ... } for FFI. Another scary-but-probably-safe transmute.
src\ios\helpers.py +1
src\ios\mobiledevice_sys.rs 0
FFI, not perfectly verified since I don't have an OS X machine to check the headers out on.
Ref: https://github.com/PanayotCankov/device.io/blob/master/idb/MobileDevice.h
Various minor const differences, a few functions missing in ref, a few likely improved definitions in places.
33 am_device_notification_callback_info has "extra" field vs reference, "subscription". iOS internal struct?
src\ios\mod.rs 0
43 unsafe { ... } for FFI. Looks safe to me.
55 technically unsound inner fn (uses ptrs)
61 Scary looking as hell, but I think this is just going Box (40) -> void* (48) -> Box (here).
62 UNDEFINED BEHAVIOR: Possible panic unwind through FFI, technically an hazard.
I would've missed this edge case but for the comment, so I'm not docking points.
Unlikely to cause severe problems, but would be worth fixing.
src\ios\platform.rs +1
src\ios\xcode.rs +1
25 Dead code not reviewed
91 I have not thoroughly audited this code signing stuff, but looks OK.
199 com.zoy.kali.Dinghy? A bit hardcoded...

src\platform\mod.rs +1
src\platform\regular_platform.rs +1
44 Since when does "regular" mean "*nix" - might not work on windows.

src\script\device.rs +1
src\script\mod.rs +1

src\ssh\device.rs 0
src\ssh\mod.rs +1

src\compiler.rs +1
475 Isn't this unbanned?
src\config.rs +1
src\device.rs +1
src\errors.rs +1
src\lib.rs +1
87 random sleep? why?
src\overlay.rs +1
src\project.rs +1
src\toolchain.rs +1
src\utils.rs +1

build.rs +1
Cargo.toml +1
Cargo.toml.orig +1

TIL

Neat loop pattern:

for (a,     b,      c) in &[
    ("a",   "b",    "c"),
    ("aa",  "bb",   "cc"),
] {
    ...
}

Sysroot paths: ndk/toolchains/llvm/prebuilt/sysroot/usr/lib/{binutils_cpu}-linux-{abi_kind}"

AVOID. This crate is unsound as fuck, and abandoned.

Alternatives:

• khronos-egl is a sounder, maintained fork.
• egli is an alternative, supposedly sound API with low and high level APIs.
• glutin_egl_sys uses gl_generator to provide low level unsafe API structs.

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/egl.md

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/foreign-types.md#050

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/foreign-types.md#040--032

foreign_type! can generate unsound code without using the unsafe keyword in client code at all.

• Invokes C FFI which may be unsafe from safe fns.
• More recent versions (0.4.0) can implement unsafe traits (Send, Sync) without the unsafe keyword at all.

Technically sound, but makes me nervous.

Opaque: Looks way too dereferencable, and would have the wrong size - that of ().
ForeignTypeRef: Unchecked pointer casts galore.

Yet another unsound ECS. Saw more 16-bit counters too.

I'm assuming GetVolumePathNameW will leave buffer null terminated if successful. Unsafe only used for sane-looking FFI, and zeroing structs.

Looks nice and solid. Well documented, well tested, code coverage, safe + sound.
Large crate - so I haven't taken the time to thoroughly audit everything - but I have at least skimmed over all code.
I've written some of my own partial DWARF parser in C++ in the past... parts look appropriately familiar.

Detail

TIL

travis-cargo

Handle basic #include s for GLSL.

Pros:

• 100% Safe/Sandboxed

Cons:

• No #file directives emitted
• No callback option for #include, must pre-register all possible includes.
• Repeat #include s simply ignored instead of interacting with preprocessor.

0.3.1

Encoders/decoders for .ico and .cur file formats.

Pros:

• Well documented and ~complete despite 0.1.0 version
• Pure Rust

Full Audit

Pros:

• Rescaling via image
• SVG rendering via resvg
• Pretty nifty and relatively comprehensive
• icon-pie's lack of multithreading makes icon-pie sound despite icon_baker's bogus Send+Sync impls

Full Audit

Pros:

• Rescaling via image
• SVG rendering via resvg
• Pretty nifty and relatively comprehensive

Cons:

• Unsound

Full Audit

0.2.0: Some minor refactoring and a lot of ugly rustfmt(?) reformatting.
0.1.5: Read all code, including skimmed every line of autogenerated rust tables. Looked for any security issues, including any obvious DoS attacks from loops. Did not try to manually verify any of the bidi/punycode/idna logic against a spec, but test coverage looks excellent.

Read all code, including skimmed every line of autogenerated rust tables. Looked for any security issues, including any obvious DoS attacks from loops. Did not try to manually verify any of the bidi/punycode/idna logic against a spec, but test coverage looks excellent.

std::time::Instant alternative that doesn't panic on wasm targets.

Pros:

• Doesn't panic!
• Just std::time::Instant on native, no performance hit or anything

Cons:

• Just std::time::Instant on native, easy to accidentally use new std APIs unavailable on wasm
• f64 repr for Instant makes me nervous
• Unusual license choice for rust projects
• Could use more test coverage

Great crate. Doesn't work with WASM.
https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/inventory.md#015

Great crate. Doesn't work with WASM.
https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/inventory.md#015

Great crate, but currently unsound. Also doesn't work with WASM.
https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/inventory.md#014

I believe this is sound, but has way too much unsafe and too few tests for my tastes.
udivmod_1e19 reduces my thoroughness and understanding to low, otherwise both would be medium or higher.

Reviewed:
benches/bench.rs +1

src/lib.rs          0
    119 mem::uninitialized (POD)
    164 impl_IntegerCommon is potentially unsound, but not public, and guarded by debug_assert!s at least.
        Would vastly prefer static_assertions though.
    196 one giant unsafe block for pointer math?  No bounds checks?  Super gross!
        Relies on $max_len being enough for $t with no bounds checking.
        Relies on d1/d2/n being unable to overflow DEC_DIGITS_LUT.
        A careful reading makes this appear technically sound, but super super sketchy.
    246 Carefully audited for impl_Integer.
    255 Carefully audited for impl_Integer.
    267 Carefully audited for impl_Integer.
    270 Carefully audited for impl_Integer.
    273 Carefully audited for impl_Integer.
    294 Again?  A careful reading makes this appear technically sound, but super super sketchy.
    338 Carefully audited for impl_Integer128.
    339 Shouldn't this technically have the cfg if U128_MAX_LEN is going to have it?
    341 Carefully audited for impl_Integer128.

tests/test.rs       +1
    Not nearly enough tests for my liking given how much unsafe code there is.
    I'd like to see every edge case of iota::write tested, they aren't.
    I'd like to see every edge case of udivmod_1e19 tested, it's not directly tested at all.

.gitignore          +1
.travis.yml         +1
Cargo.toml          +1
Cargo.toml.orig     +1
README.md           +1

Not yet fully reviewed:

src/udiv128.rs      0
    34  Correct - since high is nonzero (line 29 bails if it wasn't), leading_zeros < 64.
    35+ My eyes glazed over at this point.  I haven't verified udivmod_1e19(...).1 meets it's invariants!

TIL:
You can use \``edition2018on doc comment code blocks. You can have a public trait require a private one to "seal" it. You can specifysudo: false` in .travis.yml

Rated files were at least reviewed to thoroughness + understanding medium, but the rest was only reviewed to througuhness low.

Detail

benches/api_calls.rs

src/wrapper/descriptors/method_desc.rs

src/wrapper/java_vm/init_args.rs

src/wrapper/java_vm/vm.rs

src/wrapper/objects/global_ref.rs

src/wrapper/objects/jbytebuffer.rs

src/wrapper/objects/jlist.rs

src/wrapper/macros.rs

TIL

• Use javah to generate rust, perhaps?
• [build-dependencies]

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

src/lib.rs

1.4.0: Read diff, looks fine.
1.3.0: Read all of src, skimmed all of tests. core_lazy.rs looks a little odd, but is 100% safe code - any issues would be in it's core dependency, spin. inline_lazy.rs contains unsafe blocks... look safe, but downgrades rating to merely positive. lib.rs is just safe macros. Tests all pass.

Read all of src, skimmed all of tests. core_lazy.rs looks a little odd, but is 100% safe code - any issues would be in it's core dependency, spin. inline_lazy.rs contains unsafe blocks... look safe, but downgrades rating to merely positive. lib.rs is just safe macros. Tests all pass.

lazy_static 0.2.11 relied on unsafe muts which would have race conditions during initialization.
I recommend upgrading to a modern lazy_static 1.3.0+!

src\core_lazy.rs: +1
src\lazy.rs: -1 UNSOUND (race conditions in multithreaded init due to static mut / mut refs taken outside of call_once)
src\lib.rs: -1 UNSOUND (uses the static muts without syncronization)
src\nightly_lazy.rs: -1 UNSOUND (race conditions in multithreaded init due to static mut / mut refs taken outside of call_once)

tests*: +1 (skimmed, seem fine)

.travis.yml: +1
.appveyor.yml: +1
Cargo.toml: +1
Cargo.toml.orig: +1
README.md: +1

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/lazycell.md#1.2.1
Appears quite high quality, only the minimum necessary amount of unsafe code used.

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/lazycell.md#1.2.0
Appears quite high quality, only the minimum necessary amount of unsafe code used.

Encode/decode DWARF's variable length integer format, LEB128

Pros:

• Apache-2.0 OR MIT
• Good test coverage
• Lean

Full Audit

Encode/decode DWARF's variable length integer format, LEB128

Pros:

• Apache-2.0 OR MIT
• Good test coverage
• Lean

Full Audit

Encode/decode DWARF's variable length integer format, LEB128

Pros:

• Apache-2.0 OR MIT
• Good test coverage
• Lean

Full Audit

Encode/decode DWARF's variable length integer format, LEB128

Pros:

• Apache-2.0 OR MIT
• Good test coverage
• Lean

Full Audit

Encode/decode DWARF's variable length integer format, LEB128

Pros:

• Apache-2.0 OR MIT
• Good test coverage
• Lean

Full Audit

Encode/decode DWARF's variable length integer format, LEB128

Pros:

• Good test coverage
• Lean

Cons:

• No explicit license, prefer 0.2.x.

Full Audit

A low-boilerplate, high performance archetype based ECS.

Pros:

• Archetypes
• Low boilerplate
• MIT Licensed

Cons:

• Widespread use of 16-bit values is begging for overflows
• Some O(N) patterns I dislike, fortunately with small C.
• Not MIRI friendly in even the most trivial of examples.
• Difficult-to-vet unsafe code, strongly suspect some unsound.
• Omnipresent low-value logging
• Tons of dependencies (89)

0.1.1

src/borrows.rs

src/lib.rs

src/query.rs

src/storage.rs

Miri Example

use legion::prelude::*;

[test] // PREPEND YOUR OWN POUND SIGN
fn test() {
    let universe = Universe::new(None);
    let mut world = universe.create_world();
    world.insert_from((), vec![(1i32,)]);
}

rustup toolchain install nightly-2019-09-11
rustup component add miri --toolchain=nightly-2019-09-11
cargo +nightly-2019-09-11 miri test

error[E0080]: Miri evaluation error: trying to reborrow for SharedReadWrite, but parent tag <126591> does not have an appropriate item in the borrow stack
    |
    = note: inside call to `<legion::IterEntitySource<std::vec::IntoIter<(i32,)>, (i32,)> as legion::EntitySource>::write` at C:\Users\Mike\.cargo\registry\src\github.com-1ecc6299db9ec823\legion-0.1.1\src\lib.rs:716:29
    = note: inside call to `legion::World::insert::<(), legion::IterEntitySource<std::vec::IntoIter<(i32,)>, (i32,)>>` at C:\Users\Mike\.cargo\registry\src\github.com-1ecc6299db9ec823\legion-0.1.1\src\lib.rs:689:9
note: inside call to `legion::World::insert_from::<(), std::vec::Vec<(i32,)>>` at src\lib.rs:7:5
    --> src\lib.rs:7:5
     |
7    |     world.insert_from((), vec![(1i32,)]);
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside call to `test` at src\lib.rs:4:1
    --> src\lib.rs:4:1
     |
4    | / fn test() {
5    | |     let universe = Universe::new(None);
6    | |     let mut world = universe.create_world();
7    | |     world.insert_from((), vec![(1i32,)]);
8    | | }
     | |_^

TIL

• TypeId::of::<T>()

Utility crate... LGTM!

Internal implementation detail crate of macro_rules_attribute. Relatively trivial... LGTM.

Trivial, looks correct, looks tested. cfg!(debug_assertions) is a sane cfg.

API design is super brittle. Returning uninitialized memory seems like UB-bait.

Detail

src/lib.rs

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/microprofile.md#021
Decent seeming crate, but 98% underdocumented underfuzzed C++ that is network exposed, parses strings, rewrites x86, etc.
Might use for development but using it in production would just be begging for CVEs.

Good starting point, and my favorite Rust MIDI API so far, but likely unsound as stands.

Pros:

• Pure rust, no mucking with building building C/C++ libs like with portmidi
• To be WASM/Browser compatible (PR made).

Cons:

• Likely unsound as currently stands (transmute_copy, type punning, sketchy memalloc crate use, haven't vetted thread safety properly, ...)
• API clunky in spots (mix of known issues and easily fixed surface level stuff)
• Needs more unit test coverage if possible (are there perhaps virtual MIDI devices for windows that could be added to CI...?)

TODO:

• rating: netural: Eliminate as much sketchy unsafe as possible.
• rating: positive: Clean up API design a bit (async, saner member functions, deal with Send inconsistency, maybe make connecting not consume MidiInput s?)

Detail

src/backend/asla/mod.rs

[Full Audit]
std::num::NonZero___ equivalents

Pros:

• Works
• Can represent 0
• Clever cheap xors for implementation

Cons:

• Missing a lot of traits vs equivalent NonZero*s
• Extra pointless unsafe in new impl

[Full Audit]
std::num::NonZero___ equivalents

Pros:

• Works
• Can represent 0
• Clever cheap xors for implementation

Cons:

• Missing a lot of traits vs equivalent NonZero*s
• Extra pointless unsafe in new impl

[Full Audit]
std::num::NonZero___ equivalents

Pros:

• Works
• Can represent 0
• Clever cheap xors for implementation

Cons:

• Missing a lot of traits vs equivalent NonZero*s
• Extra pointless unsafe in new impl

Queries the OS for the number of CPU cores you have.

Pros:

• You didn't have to write it.
• Handles all that hideously platform specific nonsense for you.

Cons:

• Lots of (necessary) unsafe
• Linux cgroups support seems wildly overcomplicated.
• Lies and
  lies and
  lies and
  lies.
  Not exactly this crate's fault - the system APIs are brittle and full of edge cases.
  Multi-processor architectures are full of edge cases.

Alternatives:

• Just hardcode a reasonable number of threads for your workload!
  Spinning up threads for 64 CPU cores to all false-share a single cacheline
  because that's how many logical cores were detected isn't the right choice!
  And all your threads are probably blocked on I/O anyways!

Full Audit

Queries the OS for the number of CPU cores you have.

Pros:

• You didn't have to write it.
• Handles all that hideously platform specific nonsense for you.

Cons:

• Lots of (necessary) unsafe
• Linux cgroups support seems wildly overcomplicated.
• Lies and
  lies and
  lies and
  lies.
  Not exactly this crate's fault - the system APIs are brittle and full of edge cases.
  Multi-processor architectures are full of edge cases.

Alternatives:

• Just hardcode a reasonable number of threads for your workload!
  Spinning up threads for 64 CPU cores to all false-share a single cacheline
  because that's how many logical cores were detected isn't the right choice!
  And all your threads are probably blocked on I/O anyways!

Full Audit

Queries the OS for the number of CPU cores you have.

Pros:

• You didn't have to write it.
• Handles all that hideously platform specific nonsense for you.

Cons:

• Lots of (necessary) unsafe
• Linux cgroups support seems wildly overcomplicated.
• Lies and
  lies and
  lies and
  lies.
  Not exactly this crate's fault - the system APIs are brittle and full of edge cases.
  Multi-processor architectures are full of edge cases.

Alternatives:

• Just hardcode a reasonable number of threads for your workload!
  Spinning up threads for 64 CPU cores to all false-share a single cacheline
  because that's how many logical cores were detected isn't the right choice!
  And all your threads are probably blocked on I/O anyways!

Full Audit

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum.md#0.4.3

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum.md#0.4.1

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum.md#0.4.0

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum.md#0.3.1

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum.md#0.3.0

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum.md#0.2.3

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum.md#0.2.2

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum.md#0.2.1

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum.md#0.2.0

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum.md#0.1.1

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum.md#0.1.0

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum_derive.md#0.4.3

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum_derive.md#0.4.2

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum_derive.md#0.4.1

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum_derive.md#0.4.0

Placeholder
https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/num_enum_derive.md#0.0.0

Solid looking, 100% safe. Only concern is CI scripts downloading random URLs.

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/proc-macro-crate.md#014

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/proc-macro-crate.md#013

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/proc-macro-crate.md#012

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/proc-macro-crate.md#011

https://github.com/MaulingMonkey/rust-reviews/blob/master/reviews/proc-macro-crate.md#010

Sound, although buggy AMD hardware makes implementing CryptoRng for RdRand/RdSeed a little unnerving to me ( https://github.com/nagisa/rust_rdrand/issues/12 )

Reviewed:
benches\rdrand.rs: +1
benches\rdseed.rs: +1
benches\std.rs: +1
src\changelog.rs: +1
src\lib.rs: 0
All of $gen::try_fill_bytes::imp being unsafe is still concerning... but I believe it's sound now.
RdRand/RdSeed implement CrytoRng, which makes buggy AMD hardware concerning: https://github.com/nagisa/rust_rdrand/issues/12

Concerns reduced since 0.4.0:
https://github.com/nagisa/rust_rdrand/commit/7af432c6e315fde053d0056d1b7df893a865711a
unsafe blocks appear much larger than they need to be.

Concerns fixed since 0.4.0:
https://github.com/nagisa/rust_rdrand/commit/26a0a2f9d885fbbb8e14fa47c8a48e366cf15455
mem::uninitialized() u32s
loop_rand! uses mem::uninitialized() for $el:ty, easy to misuse! Requires unsafe{} so technically sound. Not exported.
https://github.com/nagisa/rust_rdrand/commit/7af432c6e315fde053d0056d1b7df893a865711a
$gen::try_fill_bytes: UNSOUND! word and buffer reference the same data. As both are &mut Ts, this is 100% undefined behavior.
ptr::copy_nonoverlapping: This should really use a slice copy which should be just as safe...? But maybe missing from core?
Verified vs https://www.amd.com/system/files/TechDocs/24594.pdf
is_x86_feature_detected: I have not verified this is correct.

Sound, although buggy AMD hardware makes implementing CryptoRng for RdRand/RdSeed a little unnerving to me ( https://github.com/nagisa/rust_rdrand/issues/12 )

Reviewed:
benches\rdrand.rs: +1
benches\rdseed.rs: +1
benches\std.rs: +1
src\changelog.rs: +1
src\lib.rs: 0
All of $gen::try_fill_bytes::imp being unsafe is still concerning... but I believe it's sound now.
RdRand/RdSeed implement CrytoRng, which makes buggy AMD hardware concerning: https://github.com/nagisa/rust_rdrand/issues/12

Concerns reduced since 0.4.0:
https://github.com/nagisa/rust_rdrand/commit/7af432c6e315fde053d0056d1b7df893a865711a
unsafe blocks appear much larger than they need to be.

Concerns fixed since 0.4.0:
https://github.com/nagisa/rust_rdrand/commit/26a0a2f9d885fbbb8e14fa47c8a48e366cf15455
mem::uninitialized() u32s
loop_rand! uses mem::uninitialized() for $el:ty, easy to misuse! Requires unsafe{} so technically sound. Not exported.
https://github.com/nagisa/rust_rdrand/commit/7af432c6e315fde053d0056d1b7df893a865711a
$gen::try_fill_bytes: UNSOUND! word and buffer reference the same data. As both are &mut Ts, this is 100% undefined behavior.
ptr::copy_nonoverlapping: This should really use a slice copy which should be just as safe...? But maybe missing from core?
Verified vs https://www.amd.com/system/files/TechDocs/24594.pdf
is_x86_feature_detected: I have not verified this is correct.

$gen::try_fill_bytes invokes undefined behavior (overlapping &mut u32 and &mut [u8]).

Reviewed:
benches\rdrand.rs: +1
benches\rdseed.rs: +1
benches\std.rs: +1
src\changelog.rs: +1
src\lib.rs: Concerns:
mem::uninitialized() u32s
is_x86_feature_detected: I have not verified this is correct.
loop_rand! uses mem::uninitialized() for $el:ty, easy to misuse! Requires unsafe{} so technically sound. Not exported.
$gen::try_fill_bytes: UNSOUND! word and buffer reference the same data. As both are &mut Ts, this is 100% undefined behavior.
unsafe blocks appear much larger than they need to be.
ptr::copy_nonoverlapping: This should really use a slice copy which should be just as safe...? But maybe missing from core?

$gen::try_fill_bytes invokes undefined behavior (overlapping &mut u32 and &mut [u8]): https://github.com/nagisa/rust_rdrand/issues/13. 0.5.x removed some use of uninitialized.

Reviewed:
benches\rdrand.rs: +1
benches\rdseed.rs: +1
benches\std.rs: +1
src\changelog.rs: +1
src\lib.rs: Concerns:
mem::uninitialized() u32s
is_x86_feature_detected: I have not verified this is correct.
loop_rand! uses mem::uninitialized() for $el:ty, easy to misuse! Requires unsafe{} so technically sound. Not exported.
$gen::try_fill_bytes: UNSOUND! word and buffer reference the same data. As both are &mut Ts, this is 100% undefined behavior.
unsafe blocks appear much larger than they need to be.
ptr::copy_nonoverlapping: This should really use a slice copy which should be just as safe...? But maybe missing from core?

Offset read/write with &self

Pros:

• No runtime dependencies
• Very thorough, well documented

Cons:

• File isn't ReadAt/WriteAt on windows due to seeks... reasonable but annoying
• A couple of minor API holes to be fixed

Offset read/write with &self

Pros:

• No runtime dependencies
• Very thorough, well documented

Cons:

• File isn't ReadAt/WriteAt on windows due to seeks... reasonable but annoying
• A couple of minor API holes to be fixed

Full Audit

Exposes unsound APIs, lots of unverified syscalls.

Reviewed:

src\arch*.rs: Skimmed... looks reasonable, but didn't verify correct instructions / register invalidation.

src\io\dma.rs: Some unsafe... looks correct, but not thoroughly tested.
src\io\io.rs: +1
src\io\mmio.rs: UNSOUND (can construct uninitialized() T via "safe" Mmio::new()!)
src\io\mod.rs: +1
src\io\pio.rs: Some unsafe... looks reasonable, but didn't verify correct instructions.

src\scheme\generate.sh: +1
src\scheme\mod.rs: +1
src\scheme\scheme*.rs: UNSOUND (can construct arbitrary slices from arbitrary Packet s via Scheme*::handle)

.cargo_vcs_info.json: +1
.cargo-ok: +1
.gitlab-ci.yml: +1
Cargo.toml: +1
Cargo.toml.orig: +1
LICENSE: +1
README.md: +1

Skimmed:

src\call.rs: Lots of unsafe syscalls... unverified.
src\data.rs: UNSOUND (Map deref etc.)
src\error.rs: No tests for STR_ERROR, but at least it's sound.
src\flag.rs: Sound, magic constant city, meh.
src\lib.rs: LEAKS UNSOUND TRAITS into public interface!
src\number.rs: Safe, magic constant city.
src\tests.rs: Unsafe, but only #[test]s

0.2.0-alpha: Significant refactoring, adding support for generics. LGTM?
0.1.2: Seems solid, although my syn-fu is weak, limiting my ability to review this.

Detail

TIL

let Struct { ref member, ref mut member2, .. } = to_destructure;

0.2.0: More refactoring surrounding generics.
0.2.0-alpha: Significant refactoring, adding support for generics. LGTM?
0.1.2: Seems solid, although my syn-fu is weak, limiting my ability to review this.

Detail

TIL

let Struct { ref member, ref mut member2, .. } = to_destructure;