From ea8ffdfec7911386e3bcb7cc1f0708bef5ac1f37 Mon Sep 17 00:00:00 2001 From: drh Date: Wed, 22 Jul 2009 00:35:23 +0000 Subject: [PATCH] Further simplifications to btree.c, especially the sqlite3BtreeKey() and sqlite3BtreeData() functions. New assert() statements added to verify that these routines are called correctly. (CVS 6917) FossilOrigin-Name: 96cfd079528501f6f1e658ce8a5a4e3bdea729be --- manifest | 20 ++++++------- manifest.uuid | 2 +- src/btree.c | 83 +++++++++++++++++++++++++++++---------------------- src/btree.h | 6 +++- src/vdbe.c | 10 +++++-- src/vdbeaux.c | 4 ++- 6 files changed, 73 insertions(+), 52 deletions(-) diff --git a/manifest b/manifest index 1e7fb16882..81eb63d4e7 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Change\sgetAndInitPage()\s(btree.c)\sto\suse\sonly\sPagerAcquire(),\snot\sPagerLookup()\sand\sPagerAcquire().\s(CVS\s6916) -D 2009-07-21T19:25:24 +C Further\ssimplifications\sto\sbtree.c,\sespecially\sthe\ssqlite3BtreeKey()\sand\nsqlite3BtreeData()\sfunctions.\s\sNew\sassert()\sstatements\sadded\sto\sverify\nthat\sthese\sroutines\sare\scalled\scorrectly.\s(CVS\s6917) +D 2009-07-22T00:35:24 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in df9359da7a726ccb67a45db905c5447d5c00c6ef F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -106,8 +106,8 @@ F src/auth.c 802a9439dfa0b8c208b10055cba400e82ef18025 F src/backup.c 6f1c2d9862c8a3feb7739dfcca02c1f5352e37f3 F src/bitvec.c cfbf6af5b0ababb4f06ed3e75c616dadaf47fcbd F src/btmutex.c 0f43a75bb5b8147b386e8e1c3e71ba734e3863b7 -F src/btree.c 710f95cf3816202c64a8639677a8d9691abefa83 -F src/btree.h e53a10fd31d16c60a86f03c9467a6f470aa3683b +F src/btree.c ece7e39b72fb42af1811c3388bbd0724fec4a664 +F src/btree.h 577448a890c2ab9b21e6ab74f073526184bceebe F src/btreeInt.h 1c86297e69380f6577e7ae67452597dd8d5c2705 F src/build.c 867028ee9f63f7bc8eb8d4a720bb98cf9b9a12b4 F src/callback.c cb68b21b0d4ae7d11ae0e487933bce3323784dcf @@ -204,11 +204,11 @@ F src/update.c a1bbe774bce495d62dce3df3f42a5f04c1de173a F src/utf.c 9541d28f40441812c0b40f00334372a0542c00ff F src/util.c 861d5b5c58be4921f0a254489ea94cb15f550ef8 F src/vacuum.c 3fe0eebea6d2311c1c2ab2962887d11f7a4dcfb0 -F src/vdbe.c b6fadd91f509fbcb9108f1d1a5e0985735fb315d +F src/vdbe.c 3f4b02f59287cc65583c0bba371e9c7d047abad2 F src/vdbe.h 35a648bc3279a120da24f34d9a25213ec15daf8a F src/vdbeInt.h 831c254a6eef237ef4664c8381a0137586567007 F src/vdbeapi.c 0ab8ada7260b32031ca97f338caecf0812460624 -F src/vdbeaux.c 017bd2774528220c0c7b85342ad6b114011dfb53 +F src/vdbeaux.c a1ed07f91a9d59d39e97b2844554a07c91c9b809 F src/vdbeblob.c a3f3e0e877fc64ea50165eec2855f5ada4477611 F src/vdbemem.c 50cc051619ba457d762e0f17dfe1d6c926c5c4d5 F src/vtab.c 00902f289521041712fb0293d0bf8688c7af8e48 @@ -741,7 +741,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e F tool/vdbe-compress.tcl 672f81d693a03f80f5ae60bfefacd8a349e76746 -P 716fccea58c4c217e68e04e0776e44ae39c11950 -R 1ee5a6818bfe745a7b3a3e753bd0011a -U danielk1977 -Z 0104982a535ea93c433ad3cd1f268c89 +P 0b41dfc066b60ccabbf1a9ab4db41ebcb73a2799 +R ec7248901916ae3e3f51419c6b86f672 +U drh +Z e29c38deeafef8c5fb9e4e32f65332cb diff --git a/manifest.uuid b/manifest.uuid index 750bad5576..96faec28b3 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -0b41dfc066b60ccabbf1a9ab4db41ebcb73a2799 \ No newline at end of file +96cfd079528501f6f1e658ce8a5a4e3bdea729be \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index d0ce43417d..ddfebd5681 100644 --- a/src/btree.c +++ b/src/btree.c @@ -9,7 +9,7 @@ ** May you share freely, never taking more than you give. ** ************************************************************************* -** $Id: btree.c,v 1.697 2009/07/21 19:25:24 danielk1977 Exp $ +** $Id: btree.c,v 1.698 2009/07/22 00:35:24 drh Exp $ ** ** This file implements a external (disk-based) database using BTrees. ** See the header comment on "btreeInt.h" for additional information. @@ -539,6 +539,9 @@ static void btreeClearHasContent(BtShared *pBt){ /* ** Save the current cursor position in the variables BtCursor.nKey ** and BtCursor.pKey. The cursor's state is set to CURSOR_REQUIRESEEK. +** +** The caller must ensure that the cursor is valid (has eState==CURSOR_VALID) +** prior to calling this routine. */ static int saveCursorPosition(BtCursor *pCur){ int rc; @@ -548,7 +551,7 @@ static int saveCursorPosition(BtCursor *pCur){ assert( cursorHoldsMutex(pCur) ); rc = sqlite3BtreeKeySize(pCur, &pCur->nKey); - assert( rc==SQLITE_OK ); /* Cannot fail since pCur->eState==VALID */ + assert( rc==SQLITE_OK ); /* KeySize() cannot fail */ /* If this is an intKey table, then the above call to BtreeKeySize() ** stores the integer key in pCur->nKey. In this case this value is @@ -2707,11 +2710,14 @@ static int allocateBtreePage(BtShared *, MemPage **, Pgno *, Pgno, u8); ** database so that the last page of the file currently in use ** is no longer in use. ** -** If the nFin parameter is non-zero, the implementation assumes +** If the nFin parameter is non-zero, this function assumes ** that the caller will keep calling incrVacuumStep() until ** it returns SQLITE_DONE or an error, and that nFin is the ** number of pages the database file will contain after this -** process is complete. +** process is complete. If nFin is zero, it is assumed that +** incrVacuumStep() will be called a finite amount of times +** which may or may not empty the freelist. A full autovacuum +** has nFin>0. A "PRAGMA incremental_vacuum" has nFin==0. */ static int incrVacuumStep(BtShared *pBt, Pgno nFin, Pgno iLastPg){ Pgno nFreeList; /* Number of pages still on the free-list */ @@ -2855,8 +2861,8 @@ static int autoVacuumCommit(BtShared *pBt){ invalidateAllOverflowCache(pBt); assert(pBt->autoVacuum); if( !pBt->incrVacuum ){ - Pgno nFin; /* Number of pages to be freed */ - Pgno nFree; /* Number of pages no the freelist */ + Pgno nFin; /* Number of pages in database after autovacuuming */ + Pgno nFree; /* Number of pages on the freelist initially */ Pgno nPtrmap; /* Number of PtrMap pages to be freed */ Pgno iFree; /* The next page to be freed */ int nEntry; /* Number of entries on one ptrmap page */ @@ -3441,6 +3447,17 @@ int sqlite3BtreeCloseCursor(BtCursor *pCur){ } #endif /* _MSC_VER */ +#ifndef NDEBUG /* The next routine used only within assert() statements */ +/* +** Return true if the given BtCursor is valid. A valid cursor is one +** that is currently pointing to a row in a (non-empty) table. +** This is a verification routine is used only within assert() statements. +*/ +int sqlite3BtreeCursorIsValid(BtCursor *pCur){ + return pCur && pCur->eState==CURSOR_VALID; +} +#endif /* NDEBUG */ + /* ** Set *pSize to the size of the buffer needed to hold the value of ** the key for the current entry. If the cursor is not pointing @@ -3448,47 +3465,41 @@ int sqlite3BtreeCloseCursor(BtCursor *pCur){ ** ** For a table with the INTKEY flag set, this routine returns the key ** itself, not the number of bytes in the key. +** +** The caller must position the cursor prior to invoking this routine. +** +** This routine cannot fail. It always returns SQLITE_OK. */ int sqlite3BtreeKeySize(BtCursor *pCur, i64 *pSize){ - int rc; - assert( cursorHoldsMutex(pCur) ); - rc = restoreCursorPosition(pCur); - if( rc==SQLITE_OK ){ - assert( pCur->eState==CURSOR_INVALID || pCur->eState==CURSOR_VALID ); - if( pCur->eState==CURSOR_INVALID ){ - *pSize = 0; - }else{ - getCellInfo(pCur); - *pSize = pCur->info.nKey; - } + assert( pCur->eState==CURSOR_INVALID || pCur->eState==CURSOR_VALID ); + if( pCur->eState!=CURSOR_VALID ){ + *pSize = 0; + }else{ + getCellInfo(pCur); + *pSize = pCur->info.nKey; } - return rc; + return SQLITE_OK; } /* ** Set *pSize to the number of bytes of data in the entry the -** cursor currently points to. Always return SQLITE_OK. -** Failure is not possible. If the cursor is not currently -** pointing to an entry (which can happen, for example, if -** the database is empty) then *pSize is set to 0. +** cursor currently points to. +** +** The caller must guarantee that the cursor is pointing to a non-NULL +** valid entry. In other words, the calling procedure must guarantee +** that the cursor has Cursor.eState==CURSOR_VALID. +** +** Failure is not possible. This function always returns SQLITE_OK. +** It might just as well be a procedure (returning void) but we continue +** to return an integer result code for historical reasons. */ int sqlite3BtreeDataSize(BtCursor *pCur, u32 *pSize){ - int rc; - assert( cursorHoldsMutex(pCur) ); - rc = restoreCursorPosition(pCur); - if( rc==SQLITE_OK ){ - assert( pCur->eState==CURSOR_INVALID || pCur->eState==CURSOR_VALID ); - if( pCur->eState==CURSOR_INVALID ){ - /* Not pointing at a valid entry - set *pSize to 0. */ - *pSize = 0; - }else{ - getCellInfo(pCur); - *pSize = pCur->info.nData; - } - } - return rc; + assert( pCur->eState==CURSOR_VALID ); + getCellInfo(pCur); + *pSize = pCur->info.nData; + return SQLITE_OK; } /* diff --git a/src/btree.h b/src/btree.h index 3f4bffebe4..ce44756fc7 100644 --- a/src/btree.h +++ b/src/btree.h @@ -13,7 +13,7 @@ ** subsystem. See comments in the source code for a detailed description ** of what each interface routine does. ** -** @(#) $Id: btree.h,v 1.119 2009/07/09 05:07:38 danielk1977 Exp $ +** @(#) $Id: btree.h,v 1.120 2009/07/22 00:35:24 drh Exp $ */ #ifndef _BTREE_H_ #define _BTREE_H_ @@ -185,6 +185,10 @@ int sqlite3BtreePutData(BtCursor*, u32 offset, u32 amt, void*); void sqlite3BtreeCacheOverflow(BtCursor *); void sqlite3BtreeClearCursor(BtCursor *); +#ifndef NDEBUG +int sqlite3BtreeCursorIsValid(BtCursor*); +#endif + #ifndef SQLITE_OMIT_BTREECOUNT int sqlite3BtreeCount(BtCursor *, i64 *); #endif diff --git a/src/vdbe.c b/src/vdbe.c index 08c6bd2ef4..76641c4b6d 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -43,7 +43,7 @@ ** in this file for details. If in doubt, do not deviate from existing ** commenting and indentation practices when changing or adding code. ** -** $Id: vdbe.c,v 1.872 2009/07/14 18:35:45 drh Exp $ +** $Id: vdbe.c,v 1.873 2009/07/22 00:35:24 drh Exp $ */ #include "sqliteInt.h" #include "vdbeInt.h" @@ -2055,6 +2055,7 @@ case OP_Column: { payloadSize = pC->payloadSize; zRec = (char*)pC->aRow; }else if( pC->isIndex ){ + assert( sqlite3BtreeCursorIsValid(pCrsr) ); rc = sqlite3BtreeKeySize(pCrsr, &payloadSize64); assert( rc==SQLITE_OK ); /* True because of CursorMoveto() call above */ /* sqlite3BtreeParseCellPtr() uses getVarint32() to extract the @@ -2063,8 +2064,9 @@ case OP_Column: { assert( (payloadSize64 & SQLITE_MAX_U32)==(u64)payloadSize64 ); payloadSize = (u32)payloadSize64; }else{ + assert( sqlite3BtreeCursorIsValid(pCrsr) ); rc = sqlite3BtreeDataSize(pCrsr, &payloadSize); - assert( rc==SQLITE_OK ); /* True because of CursorMoveto() call above */ + assert( rc==SQLITE_OK ); /* DataSize() cannot fail */ } }else if( pC->pseudoTable ){ /* The record is the sole entry of a pseudo-table */ @@ -3593,6 +3595,7 @@ case OP_NewRowid: { /* out2-prerelease */ if( res ){ v = 1; }else{ + assert( sqlite3BtreeCursorIsValid(pC->pCursor) ); rc = sqlite3BtreeKeySize(pC->pCursor, &v); assert( rc==SQLITE_OK ); /* Cannot fail following BtreeLast() */ if( v==MAX_ROWID ){ @@ -3879,6 +3882,7 @@ case OP_RowData: { assert( pC->pseudoTable==0 ); assert( pC->pCursor!=0 ); pCrsr = pC->pCursor; + assert( sqlite3BtreeCursorIsValid(pCrsr) ); /* The OP_RowKey and OP_RowData opcodes always follow OP_NotExists or ** OP_Rewind/Op_Next with no intervening instructions that might invalidate @@ -3899,7 +3903,7 @@ case OP_RowData: { n = (u32)n64; }else{ rc = sqlite3BtreeDataSize(pCrsr, &n); - assert( rc==SQLITE_OK ); /* True because of CursorMoveto() call above */ + assert( rc==SQLITE_OK ); /* DataSize() cannot fail */ if( n>(u32)db->aLimit[SQLITE_LIMIT_LENGTH] ){ goto too_big; } diff --git a/src/vdbeaux.c b/src/vdbeaux.c index cdafff85d3..118f4ee2b9 100644 --- a/src/vdbeaux.c +++ b/src/vdbeaux.c @@ -14,7 +14,7 @@ ** to version 2.8.7, all this code was combined into the vdbe.c source file. ** But that file was getting too big so this subroutines were split out. ** -** $Id: vdbeaux.c,v 1.476 2009/07/17 17:25:43 danielk1977 Exp $ +** $Id: vdbeaux.c,v 1.477 2009/07/22 00:35:24 drh Exp $ */ #include "sqliteInt.h" #include "vdbeInt.h" @@ -2599,6 +2599,7 @@ int sqlite3VdbeIdxRowid(sqlite3 *db, BtCursor *pCur, i64 *rowid){ ** Any corruption is detected in sqlite3BtreeParseCellPtr(), though, so ** this code can safely assume that nCellKey is 32-bits */ + assert( sqlite3BtreeCursorIsValid(pCur) ); rc = sqlite3BtreeKeySize(pCur, &nCellKey); assert( rc==SQLITE_OK ); /* pCur is always valid so KeySize cannot fail */ assert( (nCellKey & SQLITE_MAX_U32)==(u64)nCellKey ); @@ -2675,6 +2676,7 @@ int sqlite3VdbeIdxKeyCompare( BtCursor *pCur = pC->pCursor; Mem m; + assert( sqlite3BtreeCursorIsValid(pCur) ); rc = sqlite3BtreeKeySize(pCur, &nCellKey); assert( rc==SQLITE_OK ); /* pCur is always valid so KeySize cannot fail */ /* nCellKey will always be between 0 and 0xffffffff because of the say