ReviewOS

also looking at this

craft-native/craft

feat(android): getDeviceInfo in Zig, and the JNI calls it needed

#137
Merged glennmichael123 wants to merge feat/android-jni-args into main
3 files +741 -0
10 days ago

The first Android action written against the JNI layer (#136), plus the parts of that layer it turned out to need.

What the layer gained

Argument-taking calls, all in the A form ([]const jvalue). The bare CallObjectMethod is variadic in C, and calling a variadic function through a callconv(.c) pointer means reproducing the platform's argument-passing rules exactly — which arm64 and x86-64 Android do not share. The A form takes an array, so there's no ABI to reproduce and one implementation is right on both.

jvalue's size and alignment are asserted. Eight bytes wrong there misaligns every argument after the first and the call still runs — the JNI failure that looks like garbage data rather than a crash. The test passes (String, int) specifically, the getPackageInfo shape, because a reference and an int travel in different register files and it's the pair a variadic call is most likely to get wrong.

Instance field reads — GetFieldID and the object/int/long/float getters. Android exposes plain public fields on several of the classes this reads: DisplayMetrics.widthPixels, PackageInfo.versionName. There are no getters to call.

Local reference frames. Reading this one reply walks eleven Java objects — two classes, four strings, a WindowManager, a Display, a DisplayMetrics, a PackageManager, a PackageInfo — and the JVM guarantees only sixteen local slots. Overflow aborts the process rather than failing a call. The alternative is eleven defers, one of which eventually goes missing.

The action, split so the reply is testable without a device

read does the JNI work and returns a plain DeviceInfo. render turns that into bytes and touches no Java at all. Escaping, key completeness, number formatting and the float's precision are all decided in render, so a host test drives them directly — the half that genuinely needs a device is only the field wiring.

Thirteen keys is the contract. The iOS equivalent of this action once answered four of fourteen fields and reported success, handing pages undefined for screenWidth. The test parses the reply and asserts count() == 13 plus every value.

Faithful to the Kotlin, and where it isn't

appVersion / appBuildReproduce the shim's try/catch fallbacks ("unknown", 0) for a throwing getPackageInfo. A null versionName — legal, and not covered by that catch — takes the same fallback rather than emitting a JSON null a page must test for.
appBuildlongVersionCode above API 28, versionCode below. Reading the wrong one truncates a modern app's build number rather than failing. Tested past 2³¹.
Build$VERSIONWith the dollar — that's the binary name; android/os/Build/VERSION finds nothing.
getDefaultDisplayStays deprecated-but-used, as in the shim. Context.getDisplay() answers a different question on a multi-display device than this action's contract promises.
isEmulatorExactly the shim's two markers, deliberately not widened — a test asserts vbox86p is not detected, because the Kotlin doesn't detect it either.

A bug its own test caught

render wrote an unconditional leading comma, so the object opened {,"model" and stopped parsing at character two. The comment above the line argued confidently for the wrong thing. Also tested: a device name containing " — ro.product.model is settable on custom builds, and appendJsonEscaped writes no delimiters, the same trap that broke the iOS location-recording state file.

zig build test:android → 40/40, peak RSS 294 MB.

Not yet reachable — stated plainly

There is still no seam. No JNI_OnLoad, no RegisterNatives, no external fun on the Kotlin side, so read has no caller and no page can reach this. That's the next commit, and it needs the JavaVM's own invoke table, which this doesn't declare.

RegisterNatives rather than symbol-name export is the plan: JNI's Java_com_example_Foo_bar naming embeds the package, and craft's package name is templated per app — so a name-mangled export would only bind for one package.

3 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.