Add some notes about the JNI pointer-passing approach and convert a couple of potential NullPointerExceptions into appropriate C result codes. Clarify that invocation of undefined behaviour from the Java API does not (due to the addition of defensive code) mean the same thing as it does in C (e.g. no NULL pointer dereferences).

FossilOrigin-Name: 19c4778f45261006368b2d9460350fed1e55fed314c8b3e1af34cd8c3c73b7d8

ext/jni/src/c/sqlite3-jni.c

@@ -185,6 +185,8 @@
 **
 ** This use of intptr_t is the _only_ reason we require <stdint.h>
 ** which, in turn, requires building with -std=c99 (or later).
+**
+** See also: the notes for LongPtrGet_T.
 */
 #define S3JniCast_L2P(JLongAsPtr) (void*)((intptr_t)(JLongAsPtr))
 #define S3JniCast_P2L(PTR) (jlong)((intptr_t)(PTR))
@@ -1493,6 +1495,15 @@ static void * NativePointerHolder__get(JNIEnv * env, jobject jNph,
 ** the C side, because it's reportedly significantly faster. The
 ** intptr_t part here is necessary for compatibility with (at least)
 ** ARM32.
+**
+** 2023-11-09: testing has not revealed any measurable performance
+** difference between the approach of passing type T to C compared to
+** passing pointer-to-T to C, and adding support for the latter
+** everywhere requires sigificantly more code. As of this writing, the
+** older/simpler approach is being applied except for (A) where the
+** newer approach has already been applied and (B) hot-spot APIs where
+** a difference of microseconds (i.e. below our testing measurement
+** threshold) might add up.
 */
 #define LongPtrGet_T(T,JLongAsPtr) (T*)((intptr_t)(JLongAsPtr))
 #define LongPtrGet_sqlite3(JLongAsPtr) LongPtrGet_T(sqlite3,JLongAsPtr)
@@ -4674,9 +4685,9 @@ S3JniApi(sqlite3_sql(),jstring,1sql)(
 }
 
 S3JniApi(sqlite3_step(),jint,1step)(
-  JniArgsEnvClass,jobject jStmt
+  JniArgsEnvClass, jlong jpStmt
 ){
-  sqlite3_stmt * const pStmt = PtrGet_sqlite3_stmt(jStmt);
+  sqlite3_stmt * const pStmt = LongPtrGet_sqlite3_stmt(jpStmt);
   return pStmt ? (jint)sqlite3_step(pStmt) : (jint)SQLITE_MISUSE;
 }
 

ext/jni/src/c/sqlite3-jni.h

@@ -1872,10 +1872,10 @@ JNIEXPORT jint JNICALL Java_org_sqlite_jni_capi_CApi_sqlite3_1status64
 /*
  * Class:     org_sqlite_jni_capi_CApi
  * Method:    sqlite3_step
- * Signature: (Lorg/sqlite/jni/capi/sqlite3_stmt;)I
+ * Signature: (J)I
  */
 JNIEXPORT jint JNICALL Java_org_sqlite_jni_capi_CApi_sqlite3_1step
-  (JNIEnv *, jclass, jobject);
+  (JNIEnv *, jclass, jlong);
 
 /*
  * Class:     org_sqlite_jni_capi_CApi

ext/jni/src/org/sqlite/jni/annotation/NotNull.java

@@ -18,12 +18,12 @@ package org.sqlite.jni.annotation;
    null or point to closed/finalized C-side resources.
 
    <p>In the case of Java types which map directly to C struct types
-   (e.g. {@link org.sqlite.jni.sqlite3}, {@link
-   org.sqlite.jni.sqlite3_stmt}, and {@link
-   org.sqlite.jni.sqlite3_context}), a closed/finalized resource is
-   also considered to be null for purposes this annotation because the
-   C-side effect of passing such a handle is the same as if null is
-   passed.</p>
+   (e.g. {@link org.sqlite.jni.capi.sqlite3}, {@link
+   org.sqlite.jni.capi.sqlite3_stmt}, and {@link
+   org.sqlite.jni.capi.sqlite3_context}), a closed/finalized resource
+   is also considered to be null for purposes this annotation because
+   the C-side effect of passing such a handle is the same as if null
+   is passed.</p>
 
    <p>When used in the context of Java interfaces which are called
    from the C APIs, this annotation communicates that the C API will
@@ -31,12 +31,23 @@ package org.sqlite.jni.annotation;
 
    <p>Passing a null, for this annotation's definition of null, for
    any parameter marked with this annoation specifically invokes
-   undefined behavior.</p>
+   undefined behavior (see below).</p>
 
    <p>Passing 0 (i.e. C NULL) or a negative value for any long-type
    parameter marked with this annoation specifically invokes undefined
-   behavior. Such values are treated as C pointers in the JNI
-   layer.</p>
+   behavior (see below). Such values are treated as C pointers in the
+   JNI layer.</p>
+
+   <p><b>Undefined behaviour:</b> the JNI build uses the {@code
+   SQLITE_ENABLE_API_ARMOR} build flag, meaning that the C code
+   invoked with invalid NULL pointers and the like will not invoke
+   undefined behavior in the conventional C sense, but may, for
+   example, return result codes which are not documented for the
+   affected APIs or may otherwise behave unpredictably. In no known
+   cases will such arguments result in C-level code dereferencing a
+   NULL pointer or accessing out-of-bounds (or otherwise invalid)
+   memory. In other words, they may cause unexpected behavior but
+   should never cause an outright crash or security issue.</p>
 
    <p>Note that the C-style API does not throw any exceptions on its
    own because it has a no-throw policy in order to retain its C-style
@@ -48,7 +59,7 @@ package org.sqlite.jni.annotation;
    code.</p>
 
    <p>This annotation is solely for the use by the classes in the
-   org.sqlite package and subpackages, but is made public so that
+   org.sqlite.jni package and subpackages, but is made public so that
    javadoc will link to it from the annotated functions. It is not
    part of the public API and client-level code must not rely on
    it.</p>

ext/jni/src/org/sqlite/jni/capi/CApi.java

@@ -1734,20 +1734,24 @@ public final class CApi {
     @NotNull OutputPointer.Int64 pHighwater, boolean reset
   );
 
-  public static native int sqlite3_step(@NotNull sqlite3_stmt stmt);
+  private static native int sqlite3_step(@NotNull long ptrToStmt);
+
+  public static int sqlite3_step(@NotNull sqlite3_stmt stmt){
+    return null==stmt ? SQLITE_MISUSE : sqlite3_step(stmt.getNativePointer());
+  }
 
   public static native boolean sqlite3_stmt_busy(@NotNull sqlite3_stmt stmt);
 
   private static native int sqlite3_stmt_explain(@NotNull long ptrToStmt, int op);
 
   public static int sqlite3_stmt_explain(@NotNull sqlite3_stmt stmt, int op){
-    return sqlite3_stmt_explain(stmt.getNativePointer(), op);
+    return null==stmt ? SQLITE_MISUSE : sqlite3_stmt_explain(stmt.getNativePointer(), op);
   }
 
   private static native int sqlite3_stmt_isexplain(@NotNull long ptrToStmt);
 
   public static int sqlite3_stmt_isexplain(@NotNull sqlite3_stmt stmt){
-    return sqlite3_stmt_isexplain(stmt.getNativePointer());
+    return null==stmt ? 0 : sqlite3_stmt_isexplain(stmt.getNativePointer());
   }
 
   public static native boolean sqlite3_stmt_readonly(@NotNull sqlite3_stmt stmt);

manifest

@@ -1,5 +1,5 @@
-C Two\smore\sJNI\sbuild\sfixes\sfor\sWindows/MinGW,\sreported\sin\s[forum:4f949edc312d2a75|forum\spost\s4f949edc312d2a75].
-D 2023-11-09T12:01:02.729
+C Add\ssome\snotes\sabout\sthe\sJNI\spointer-passing\sapproach\sand\sconvert\sa\scouple\sof\spotential\sNullPointerExceptions\sinto\sappropriate\sC\sresult\scodes.\sClarify\sthat\sinvocation\sof\sundefined\sbehaviour\sfrom\sthe\sJava\sAPI\sdoes\snot\s(due\sto\sthe\saddition\sof\sdefensive\scode)\smean\sthe\ssame\sthing\sas\sit\sdoes\sin\sC\s(e.g.\sno\sNULL\spointer\sdereferences).
+D 2023-11-09T12:48:54.107
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -241,9 +241,9 @@ F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a3
 F ext/jni/GNUmakefile d984ea9c4e3536188f9d663120db8fb97b83329f4b8864bd88f75ebe581b8b8b
 F ext/jni/README.md ef9ac115e97704ea995d743b4a8334e23c659e5534c3b64065a5405256d5f2f4
 F ext/jni/jar-dist.make 030aaa4ae71dd86e4ec5e7c1e6cd86f9dfa47c4592c070d2e35157e42498e1fa
-F ext/jni/src/c/sqlite3-jni.c 6b95974189d7cc394afbe15507050f1d174170a65be5a4dad201ab11f0a9777a
-F ext/jni/src/c/sqlite3-jni.h 18925c56d6664fdec081c56daf3b2ffa0e0ff6b9b128b9f39b84862f34ba0601
-F ext/jni/src/org/sqlite/jni/annotation/NotNull.java a99341e88154e70447596b1af6a27c586317df41a7e0f246fd41370cd7b723b2
+F ext/jni/src/c/sqlite3-jni.c 3774703e5865e7ff776b762de5386af8aa703e569bbb3a85c423c3f8473a3c26
+F ext/jni/src/c/sqlite3-jni.h 489044eae9fc6c2d62c1621e41594adf7bfcd4049b514a202c4aa6fe5c1ef405
+F ext/jni/src/org/sqlite/jni/annotation/NotNull.java 02091a8112e33389f1c160f506cd413168c8dfacbeda608a4946c6e3557b7d5a
 F ext/jni/src/org/sqlite/jni/annotation/Nullable.java 0b1879852707f752512d4db9d7edd0d8db2f0c2612316ce1c832715e012ff6ba
 F ext/jni/src/org/sqlite/jni/annotation/package-info.java 977b374aed9d5853cbf3438ba3b0940abfa2ea4574f702a2448ee143b98ac3ca
 F ext/jni/src/org/sqlite/jni/capi/AbstractCollationCallback.java 1afa90d3f236f79cc7fcd2497e111992644f7596fbc8e8bcf7f1908ae00acd6c
@@ -251,7 +251,7 @@ F ext/jni/src/org/sqlite/jni/capi/AggregateFunction.java 0b72cdff61533b564d65b63
 F ext/jni/src/org/sqlite/jni/capi/AuthorizerCallback.java c045a5b47e02bb5f1af91973814a905f12048c428a3504fbc5266d1c1be3de5a
 F ext/jni/src/org/sqlite/jni/capi/AutoExtensionCallback.java 74cc4998a73d6563542ecb90804a3c4f4e828cb4bd69e61226d1a51f4646e759
 F ext/jni/src/org/sqlite/jni/capi/BusyHandlerCallback.java 7b8e19810c42b0ad21a04b5d8c804b32ee5905d137148703f16a75b612c380ca
-F ext/jni/src/org/sqlite/jni/capi/CApi.java 16a28138c3c25f33356193970644389ff8ebc0720499549653934b2529c8d1dd
+F ext/jni/src/org/sqlite/jni/capi/CApi.java 2917e2c608ac52ebe30fbcde2b520c6ea3bc99e734619dfdedd072b8e956b84f
 F ext/jni/src/org/sqlite/jni/capi/CallbackProxy.java 57e2d275dcebe690b1fc1f3d34eb96879b2d7039bce30b563aee547bf45d8a8b
 F ext/jni/src/org/sqlite/jni/capi/CollationCallback.java e29bcfc540fdd343e2f5cca4d27235113f2886acb13380686756d5cabdfd065a
 F ext/jni/src/org/sqlite/jni/capi/CollationNeededCallback.java 5bfa226a8e7a92e804fd52d6e42b4c7b875fa7a94f8e2c330af8cc244a8920ab
@@ -2139,8 +2139,8 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 1c98d46d60ef1494bd8b7561c7d0cd5aafc178201a6f1f0da25dea6140b91cd0
-R 17c07891ab0f75a8281c2b918fd5c523
+P a3f9c39086e582e16ca15647961956b3c28d038655d3b43d4b94bd306fbec1a4
+R d00dc0a6ea6feccf2aee95fe4c483ba0
 U stephan
-Z 19240fed0f90b5e1367576a460d156b0
+Z 4aa2d438a8d23b9c44b1a48729f3cdde
 # Remove this line to create a well-formed Fossil manifest.

manifest.uuid

@@ -1 +1 @@
-a3f9c39086e582e16ca15647961956b3c28d038655d3b43d4b94bd306fbec1a4
+19c4778f45261006368b2d9460350fed1e55fed314c8b3e1af34cd8c3c73b7d8