From: drh <> Date: Tue, 25 Jul 2023 20:26:47 +0000 (+0000) Subject: Clarify ownership of the various objects involved in parsing JSON. X-Git-Tag: version-3.43.0~114^2~5^2~2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=59b8e666f669a97e7d62fd1a6e24daed446f027e;p=thirdparty%2Fsqlite.git Clarify ownership of the various objects involved in parsing JSON. FossilOrigin-Name: afe02a398a16d51bd7482b6fbe2fbd15d9ac4fd9cdbc9d2bf81f38b3391fc567 --- diff --git a/manifest b/manifest index 09e5a7cfff..437038d380 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Incremental\simprovements\sto\sJSON\sparsing\s-\strying\sto\sfold\sin\sthe\sRCStr\sobject. -D 2023-07-25T18:28:03.341 +C Clarify\sownership\sof\sthe\svarious\sobjects\sinvolved\sin\sparsing\sJSON. +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 -R 2b5df60a4958d2a70ce11e3569ae2bab +P 4cb15d934a85ebc290fe6dd8cd3bd47b159561ca75d72bbffef30b9ea4623b09 +R cbb913300d23d0635674a5fded1ff5ab U drh -Z d10e127820cbbcdc4a84e1b86eb42adb +Z f943de02bc9e737c29b8eb0aa52b28c4 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index a811037656..f222cbf323 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -4cb15d934a85ebc290fe6dd8cd3bd47b159561ca75d72bbffef30b9ea4623b09 \ No newline at end of file +afe02a398a16d51bd7482b6fbe2fbd15d9ac4fd9cdbc9d2bf81f38b3391fc567 \ No newline at end of file diff --git a/src/json.c b/src/json.c index 194d0421eb..6cb429f407 100644 --- a/src/json.c +++ b/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; iKeynJson==nJson - && (p->isMod==0 || bUnchng==0) - && memcmp(p->zJson,zJson,nJson)==0 - ){ - p->nErr = 0; - pMatch = p; - }else if( p->iHoldiHold; - 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, -** and also register the new parse so that it will be available for +** is no longer valid, parse the JSON again and return the new parse. +** 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 -** JsonParse.nErr non-zero. If the caller invokes this routine with -** pErrCtx==0 and it gets back a JsonParse with nErr!=0, then the caller -** is responsible for invoking jsonParseFree() on the returned value. -** But the caller may invoke jsonParseFree() *only* if pParse->nErr!=0. +** The returned pointer (if it is not NULL) is owned by the cache in +** most cases, not the caller. The caller does NOT need to invoke +** jsonParseFree(), in most cases. +** +** 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; iKeynErr = 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]; - memcpy((char*)p->zJson, zJson, nJson+1); - if( jsonParse(p, pErrCtx, p->zJson) ){ + p->zJson = sqlite3RCStrNew( nJson ); + if( p->zJson==0 ){ + 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; } - sqlite3_free(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; izJson; + const char *z = (const char*)sqlite3_value_text(argv[0]); for(i=0; iiErr && 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); diff --git a/src/printf.c b/src/printf.c index 7b84288168..222e70be29 100644 --- a/src/printf.c +++ b/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;