Clarify ownership of the various objects involved in parsing JSON.

FossilOrigin-Name: afe02a398a16d51bd7482b6fbe2fbd15d9ac4fd9cdbc9d2bf81f38b3391fc567

manifest

@@ -1,5 +1,5 @@
-C Incremental\simprovements\sto\sJSON\sparsing\s-\strying\sto\sfold\sin\sthe\sRCStr\sobject.
+C Clarify\sownership\sof\sthe\svarious\sobjects\sinvolved\sin\sparsing\sJSON.
-D 2023-07-25T18:28:03.341
+D 2023-07-25T20:26:47.054
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -598,7 +598,7 @@ F src/hash.h 3340ab6e1d13e725571d7cee6d3e3135f0779a7d8e76a9ce0a85971fa3953c51
 F src/hwtime.h f9c2dfb84dce7acf95ce6d289e46f5f9d3d1afd328e53da8f8e9008e3b3caae6
 F src/in-operator.md 10cd8f4bcd225a32518407c2fb2484089112fd71
 F src/insert.c 3f0a94082d978bbdd33c38fefea15346c6c6bffb70bc645a71dc0f1f87dd3276
-F src/json.c 28fe8ed9e63293fd5708d129d4edd4a634887aabb0364685195974e35ee10762
+F src/json.c 97f8c20c1cfefbc54e2b8b2143a24aae4993c12de6e74764af443e033b01b013
 F src/legacy.c d7874bc885906868cd51e6c2156698f2754f02d9eee1bae2d687323c3ca8e5aa
 F src/loadext.c 176d6b2cb18a6ad73b133db17f6fc351c4d9a2d510deebdb76c22bde9cfd1465
 F src/main.c 512b1d45bc556edf4471a845afb7ba79e64bd5b832ab222dc195c469534cd002
@@ -634,7 +634,7 @@ F src/pcache1.c 602acb23c471bb8d557a6f0083cc2be641d6cafcafa19e481eba7ef4c9ca0f00
 F src/pragma.c 37b8fb02d090262280c86e1e2654bf59d8dbfbfe8dc6733f2b968a11374c095a
 F src/pragma.h e690a356c18e98414d2e870ea791c1be1545a714ba623719deb63f7f226d8bb7
 F src/prepare.c d6c4354f8ea0dc06962fbabc4b68c4471a45276a2918c929be00f9f537f69eb1
-F src/printf.c 21e410b0a3904dddb39ef1b245cfb991302022a6f0fc16f0a8a13539d6d2f713
+F src/printf.c 1406ade1451adfa4374d8e9bbb8606109742c1216f5dbc95e011bf721fd91365
 F src/random.c 606b00941a1d7dd09c381d3279a058d771f406c5213c9932bbd93d5587be4b9c
 F src/resolve.c 37953a5f36c60bea413c3c04efcd433b6177009f508ef2ace0494728912fe2e9
 F src/rowset.c 8432130e6c344b3401a8874c3cb49fefe6873fec593294de077afea2dce5ec97
@@ -2044,8 +2044,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 c456e4a8999066cd96246327101b3cca78294511a71a2ac07939bb702bfcb5f4
+P 4cb15d934a85ebc290fe6dd8cd3bd47b159561ca75d72bbffef30b9ea4623b09
-R 2b5df60a4958d2a70ce11e3569ae2bab
+R cbb913300d23d0635674a5fded1ff5ab
 U drh
-Z d10e127820cbbcdc4a84e1b86eb42adb
+Z f943de02bc9e737c29b8eb0aa52b28c4
 # Remove this line to create a well-formed Fossil manifest.

manifest.uuid

@@ -1 +1 @@
-4cb15d934a85ebc290fe6dd8cd3bd47b159561ca75d72bbffef30b9ea4623b09
+afe02a398a16d51bd7482b6fbe2fbd15d9ac4fd9cdbc9d2bf81f38b3391fc567

src/json.c

@@ -171,6 +171,17 @@ struct JsonParse {
 ** Utility routines for dealing with JsonString objects
 **************************************************************************/
 
+#if 0
+/*
+** This is a destructor for JSON strings.  We make it a separate function
+** so that the sqlite3ValueIsOfClass() function can be used to unambiguously
+** identify sqlite3_value objects that are known JSON strings.
+*/
+static void jsonStringClass(void *p){
+  sqlite3RCStrUnref((char*)p);
+}
+#endif
+
 /* Set the JsonString object to an empty string
 */
 static void jsonZero(JsonString *p){
@@ -1700,16 +1711,27 @@ json_parse_restart:
 ** pParse.
 **
 ** pParse is uninitialized when this routine is called.
+**
+** pParse->nJPRef set to 1.  The caller becomes the owner of the 
+** the JsonParse object.
+**
+** pParse->bOwnsJson is to bTakeJson.  If bTakeJson is 1, the newly initialized
+** JsonParse object will become the own the zJson input string.  If bTakeJson
+** is 0, then the caller is responsible for preserving zJson for the lifetime
+** of the JsonParse object.
 */
 static int jsonParse(
   JsonParse *pParse,           /* Initialize and fill this JsonParse object */
   sqlite3_context *pCtx,       /* Report errors here */
-  char *zJson                  /* Input JSON text to be parsed */
+  char *zJson,                 /* Input JSON text to be parsed */
+  int bTakeJson                /* Assume ownership of zJson if true */
 ){
   int i;
   memset(pParse, 0, sizeof(*pParse));
   if( zJson==0 ) return 1;
   pParse->zJson = zJson;
+  pParse->bOwnsJson = bTakeJson;
+  pParse->nJPRef = 1;
   i = jsonParseValue(pParse, 0);
   if( pParse->oom ) i = -1;
   if( i>0 ){
@@ -1738,120 +1760,6 @@ static int jsonParse(
   return 0;
 }
 
-#if 0
-/*
-** This is a destructor for JSON strings.  We make it a separate function
-** so that the sqlite3ValueIsOfClass() function can be used to unambiguously
-** identify sqlite3_value objects that are known JSON strings.
-*/
-static void jsonClass(void *p){
-  sqlite3RCStrUnref((char*)p);
-}
-#endif
-
-#if 0
-/*
-** Process SQL function argument pJson as a JSON string.  Return a pointer
-** to its parse.
-**
-** If any error is encountered, return a NULL pointer and leave an error
-** message in pCtx.
-*/
-static JsonParse *jsonParseFromFunctionArg(
-  sqlite3_value *pJson,       /* An SQL function argument containing JSON */
-  sqlite3_context *pCtx,      /* For accessing the cache */
-  sqlite3_context *pErrCtx,   /* For reporting errors */
-  int bUnchng                 /* Only accept cached parse that are unchanged */
-){
-  JsonParse *pParse;
-  char *zJson;
-  int nJson;
-  JsonParse *pMatch = 0;
-  int iKey;
-  int iMinKey = 0;
-  u32 iMinHold = 0xffffffff;
-  u32 iMaxHold = 0;
-
-  char *zJson = (char*)sqlite3_value_text(pJson);
-  int nJson = sqlite3_value_bytes(pJson);
-  if( zJson==0 ) goto json_parse_value_nomem;
-  if( sqlite3ValueIsOfClass(pJson, jsonClass) ){
-    assert( zJson!=0 );
-    (void)sqlite3RCStrRef(zJson);
-    pParse = sqlite3RCStrGetAttachment(zJson, (void(*)(void*)jsonParseFree);
-    if( pParse ){
-      return pParse;
-    }
-  }else{
-    char *z = zJson;
-    zJson = sqlite3RCStrNew( nJson );
-    if( zJson==0 ) goto json_parse_value_nomem;
-    assert( strlen(z)==nJson );
-    memcpy(zJson, z, nJson+1);
-  }
-
-  /* At this point zJson is an RCStr object that does not yet have a
-  ** parse.  This procedure is holding its own reference to that zJson
-  ** and needs to release it, or hand it off, prior to returning.
-  **
-  ** The next step is to try to find a parse of the JSON that already
-  ** exists in cache.
-  */
-  for(iKey=0; iKey<JSON_CACHE_SZ; iKey++){
-    JsonParse *p;
-    p = (JsonParse*)sqlite3_get_auxdata(pCtx, JSON_CACHE_ID+iKey);
-    if( p==0 ){
-      iMinKey = iKey;
-      break;
-    }
-    if( pMatch==0
-     && p->nJson==nJson
-     && (p->isMod==0 || bUnchng==0)
-     && memcmp(p->zJson,zJson,nJson)==0
-    ){
-      p->nErr = 0;
-      pMatch = p;
-    }else if( p->iHold<iMinHold ){
-      iMinHold = p->iHold;
-      iMinKey = iKey;
-    }
-    if( p->iHold>iMaxHold ){
-      iMaxHold = p->iHold;
-    }
-  }
-
-  if( pMatch ){
-    pParse = pMatch;
-    pParse->iHold = iMaxHold+1;
-  }else{
-    /* No parse of zJson could be found in cache.  So parse it afresh.
-    */
-    pParse = sqlite3_malloc64( sizeof(JsonParse) );
-    if( pParse==0 ) goto json_parse_value_nomem;
-    if( jsonParse(pParse, pCtx, zJson) ){
-      sqlite3_free(pParse);
-      sqlite3RCStrUnref(zJson);
-      return 0;
-    }
-    pParse->bOwnsJson = 1;
-    pParse->nJson = nJson;
-    pParse->nJPRef = 1;
-    pParse->iHold = iMaxHold+1;
-    sqlite3_set_auxdata(pCtx, JSON_CACHE_ID+iMinKey, pParse,
-                        (void(*)(void*))jsonParseFree);
-    pParse = (JsonParse*)sqlite3_get_auxdata(pCtx, JSON_CACHE_ID+iMinKey);
-  }
-  if( pParse ){
-    pParse->nJPRef++;  /* The caller will own this object */
-  }
-  return pParse;
-
-json_parse_value_nomem:
-  if( zJson ) sqlite3RCStrUnref(zJson);
-  if( pErrCtx ) sqlite3_result_error_nomem(pCtx);
-  return 0;
-}
-#endif
 
 /* Mark node i of pParse as being a child of iParent.  Call recursively
 ** to fill in all the descendants of node i.
@@ -1905,18 +1813,28 @@ static int jsonParseFindParents(JsonParse *pParse){
 ** Obtain a complete parse of the JSON found in the first argument
 ** of the argv array.  Use the sqlite3_get_auxdata() cache for this
 ** parse if it is available.  If the cache is not available or if it
-** is no longer valid, parse the JSON again and return the new parse,
+** is no longer valid, parse the JSON again and return the new parse.
-** and also register the new parse so that it will be available for
+** Also register the new parse so that it will be available for
 ** future sqlite3_get_auxdata() calls.
 **
 ** If an error occurs and pErrCtx!=0 then report the error on pErrCtx
 ** and return NULL.
 **
-** If an error occurs and pErrCtx==0 then return the Parse object with
+** The returned pointer (if it is not NULL) is owned by the cache in
-** JsonParse.nErr non-zero.  If the caller invokes this routine with
+** most cases, not the caller.  The caller does NOT need to invoke
-** pErrCtx==0 and it gets back a JsonParse with nErr!=0, then the caller
+** jsonParseFree(), in most cases.
-** is responsible for invoking jsonParseFree() on the returned value.
+**
-** But the caller may invoke jsonParseFree() *only* if pParse->nErr!=0.
+** Except, if an error occurs and pErrCtx==0 then return the JsonParse
+** object with JsonParse.nErr non-zero and the caller will own the JsonParse
+** object.  In that case, it will be the responsibility of the caller to
+** invoke jsonParseFree().  To summarize:
+**
+**   pErrCtx!=0 || p->nErr==0      ==>   Return value p is owned by the
+**                                       cache.  Call does not need to
+**                                       free it.
+**
+**   pErrCtx==0 && p->nErr!=0      ==>   Return value is owned by the caller
+**                                       and so the caller must free it.
 */
 static JsonParse *jsonParseCached(
   sqlite3_context *pCtx,
@@ -1931,6 +1849,7 @@ static JsonParse *jsonParseCached(
   int iMinKey = 0;
   u32 iMinHold = 0xffffffff;
   u32 iMaxHold = 0;
+
   if( zJson==0 ) return 0;
   for(iKey=0; iKey<JSON_CACHE_SZ; iKey++){
     p = (JsonParse*)sqlite3_get_auxdata(pCtx, JSON_CACHE_ID+iKey);
@@ -1955,27 +1874,35 @@ static JsonParse *jsonParseCached(
   if( pMatch ){
     pMatch->nErr = 0;
     pMatch->iHold = iMaxHold+1;
+    assert( pMatch->nJPRef>0 ); /* pMatch is owned by the cache */
     return pMatch;
   }
-  p = sqlite3_malloc64( sizeof(*p) + nJson + 1 );
+  p = sqlite3_malloc64( sizeof(*p) );
   if( p==0 ){
     sqlite3_result_error_nomem(pCtx);
     return 0;
   }
   memset(p, 0, sizeof(*p));
-  p->zJson = (char*)&p[1];
+  p->zJson = sqlite3RCStrNew( nJson );
-  memcpy((char*)p->zJson, zJson, nJson+1);
+  if( p->zJson==0 ){
-  if( jsonParse(p, pErrCtx, p->zJson) ){
-    if( pErrCtx==0 ){
-      p->nErr = 1;
-      return p;
-    }
     sqlite3_free(p);
+    sqlite3_result_error_nomem(pCtx);
+    return 0;
+  }
+  memcpy(p->zJson, zJson, nJson);
+  p->zJson[nJson] = 0;
+  if( jsonParse(p, pErrCtx, p->zJson, 1) ){
+    if( pErrCtx==0 ){
+      p->nErr = 1;
+      assert( p->nJPRef==1 ); /* Caller will own the new JsonParse object p */
+      return p;
+    }
+    jsonParseFree(p);
     return 0;
   }
-  p->nJPRef = 1;
   p->nJson = nJson;
   p->iHold = iMaxHold+1;
+  /* Transfer ownership of the new JsonParse to the cache */
   sqlite3_set_auxdata(pCtx, JSON_CACHE_ID+iMinKey, p,
                       (void(*)(void*))jsonParseFree);
   return (JsonParse*)sqlite3_get_auxdata(pCtx, JSON_CACHE_ID+iMinKey);
@@ -2357,7 +2284,7 @@ static void jsonParseFunc(
   u32 i;
 
   assert( argc==1 );
-  if( jsonParse(&x, ctx, (char*)sqlite3_value_text(argv[0])) ) return;
+  if( jsonParse(&x, ctx, (char*)sqlite3_value_text(argv[0]), 0) ) return;
   jsonParseFindParents(&x);
   jsonInit(&s, ctx);
   for(i=0; i<x.nNode; i++){
@@ -2681,8 +2608,8 @@ static void jsonPatchFunc(
   JsonNode *pResult;   /* The result of the merge */
 
   UNUSED_PARAMETER(argc);
-  if( jsonParse(&x, ctx, (char*)sqlite3_value_text(argv[0])) ) return;
+  if( jsonParse(&x, ctx, (char*)sqlite3_value_text(argv[0]), 0) ) return;
-  if( jsonParse(&y, ctx, (char*)sqlite3_value_text(argv[1])) ){
+  if( jsonParse(&y, ctx, (char*)sqlite3_value_text(argv[1]), 0) ){
     jsonParseReset(&x);
     return;
   }
@@ -2758,7 +2685,7 @@ static void jsonRemoveFunc(
   u32 i;
 
   if( argc<1 ) return;
-  if( jsonParse(&x, ctx, (char*)sqlite3_value_text(argv[0])) ) return;
+  if( jsonParse(&x, ctx, (char*)sqlite3_value_text(argv[0]), 0) ) return;
   assert( x.nNode );
   for(i=1; i<(u32)argc; i++){
     zPath = (const char*)sqlite3_value_text(argv[i]);
@@ -2880,7 +2807,7 @@ static void jsonReplaceFunc(
     jsonWrongNumArgs(ctx, "replace");
     return;
   }
-  if( jsonParse(&x, ctx, (char*)sqlite3_value_text(argv[0])) ) return;
+  if( jsonParse(&x, ctx, (char*)sqlite3_value_text(argv[0]), 0) ) return;
   assert( x.nNode );
   for(i=1; i<(u32)argc; i+=2){
     zPath = (const char*)sqlite3_value_text(argv[i]);
@@ -2926,7 +2853,7 @@ static void jsonSetFunc(
     jsonWrongNumArgs(ctx, bIsSet ? "set" : "insert");
     return;
   }
-  if( jsonParse(&x, ctx, (char*)sqlite3_value_text(argv[0])) ) return;
+  if( jsonParse(&x, ctx, (char*)sqlite3_value_text(argv[0]), 0) ) return;
   assert( x.nNode );
   for(i=1; i<(u32)argc; i+=2){
     zPath = (const char*)sqlite3_value_text(argv[i]);
@@ -3046,7 +2973,7 @@ static void jsonErrorFunc(
   }else{
     int n = 1;
     u32 i;
-    const char *z = p->zJson;
+    const char *z = (const char*)sqlite3_value_text(argv[0]);
     for(i=0; i<p->iErr && ALWAYS(z[i]); i++){
       if( (z[i]&0xc0)!=0x80 ) n++;
     }
@@ -3639,7 +3566,7 @@ static int jsonEachFilter(
   p->zJson = sqlite3_malloc64( n+1 );
   if( p->zJson==0 ) return SQLITE_NOMEM;
   memcpy(p->zJson, z, (size_t)n+1);
-  if( jsonParse(&p->sParse, 0, p->zJson) ){
+  if( jsonParse(&p->sParse, 0, p->zJson, 0) ){
     int rc = SQLITE_NOMEM;
     if( p->sParse.oom==0 ){
       sqlite3_free(cur->pVtab->zErrMsg);

src/printf.c

@@ -1430,7 +1430,7 @@ int sqlite3RCStrIsWriteable(char *z){
 ** This routine returns 0 on an OOM.
 */
 char *sqlite3RCStrNew(u64 N){
-  RCStr *p = sqlite3_malloc64( N + sizeof(*p) );
+  RCStr *p = sqlite3_malloc64( N + sizeof(*p) + 1 );
   if( p==0 ) return 0;
   p->nRCRef = 1;
   p->xFree = 0;