Change getAndInitPage() (btree.c) to use only PagerAcquire(), not PagerLookup() and PagerAcquire(). (CVS 6916)

FossilOrigin-Name: 0b41dfc066b60ccabbf1a9ab4db41ebcb73a2799

manifest

@@ -1,5 +1,5 @@
-C Additional\ssimplifications\sto\sbtree.c\sin\ssupport\sof\scoverage\stesting.\s(CVS\s6915)
+C Change\sgetAndInitPage()\s(btree.c)\sto\suse\sonly\sPagerAcquire(),\snot\sPagerLookup()\sand\sPagerAcquire().\s(CVS\s6916)
-D 2009-07-21T19:02:21
+D 2009-07-21T19:25:24
 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0
 F Makefile.in df9359da7a726ccb67a45db905c5447d5c00c6ef
 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654
@@ -106,7 +106,7 @@ F src/auth.c 802a9439dfa0b8c208b10055cba400e82ef18025
 F src/backup.c 6f1c2d9862c8a3feb7739dfcca02c1f5352e37f3
 F src/bitvec.c cfbf6af5b0ababb4f06ed3e75c616dadaf47fcbd
 F src/btmutex.c 0f43a75bb5b8147b386e8e1c3e71ba734e3863b7
-F src/btree.c abc2ccac9f9cf605888b48689e5886301c0f25c0
+F src/btree.c 710f95cf3816202c64a8639677a8d9691abefa83
 F src/btree.h e53a10fd31d16c60a86f03c9467a6f470aa3683b
 F src/btreeInt.h 1c86297e69380f6577e7ae67452597dd8d5c2705
 F src/build.c 867028ee9f63f7bc8eb8d4a720bb98cf9b9a12b4
@@ -147,10 +147,10 @@ F src/os_common.h 8c61457df58f1a4bd5f5adc3e90e01b37bf7afbc
 F src/os_os2.c bed77dc26e3a95ce4a204936b9a1ca6fe612fcc5
 F src/os_unix.c cdb2a08b9ce4aa13b3f7b91d4dd60fb48be9f56a
 F src/os_win.c 725c38a524d168ce280446ad8761d731bc516405
-F src/pager.c ed7cdaac02878f7b74d4de01b36520e5505c9af6
+F src/pager.c 9838393b4a5118eb935ac0939621177157a38bc1
-F src/pager.h 5aec418bf99f568b92ae82816a1463400513726d
+F src/pager.h 5bd96ed838e4156e0effa5ffe746bce4c0112c24
 F src/parse.y bcd46d43fbd23a22b8c020a3eb1806b794794ed5
-F src/pcache.c 7d27635a4bd03c81ce848cb186fea6192706fa1a
+F src/pcache.c 1dae135b70a029f81ed66f6e9b5d0db91480d5d0
 F src/pcache.h 9b927ccc5a538e31b4c3bc7eec4f976db42a1324
 F src/pcache1.c 6dc833c89feac405dd8b4858232c97e679f182ec
 F src/pragma.c 9eb44ac1d3dc1ac3ea4f444abe1a10ae8acaa16c
@@ -169,7 +169,7 @@ F src/status.c 237b193efae0cf6ac3f0817a208de6c6c6ef6d76
 F src/table.c cc86ad3d6ad54df7c63a3e807b5783c90411a08d
 F src/tclsqlite.c e18e5013dc6bca9f25e6022fbe17ba3ccb821f95
 F src/test1.c c8f9358879876660b721369f576bf6e4ac5b9210
-F src/test2.c d7d7907cec3ed918b3b35e8e44fb3de3709d9509
+F src/test2.c ffb1649b7a33a0acd5bd1a98a376b9c104daa279
 F src/test3.c 2445c2beb5e7a0c91fd8136dc1339ec369a24898
 F src/test4.c f79ab52d27ff49b784b631a42e2ccd52cfd5c84c
 F src/test5.c 162a1cea2105a2c460a3f39fa6919617b562a288
@@ -291,7 +291,7 @@ F test/corrupt7.test 8bfb08842642b1a598a915b52a7f51fbc0289b62
 F test/corrupt8.test 9992ef7f67cefc576b92373f6bf5ab8775280f51
 F test/corrupt9.test 4aa1cb1ef091cb0e13e89a819c72911631b5176a
 F test/corruptA.test 99e95620b980161cb3e79f06a884a4bb8ae265ff
-F test/corruptB.test 267be7fbb7bdb89df40007178437ebdd0cd9c174
+F test/corruptB.test 66b4544104dd03d0f33ea69ddac3fa4a682cd3c2
 F test/corruptC.test 691ed070baef5e1345939caadf270a52837a5064
 F test/corruptD.test 3ae6e2dc6e2226c6935a8a40d4b5ee3eba75f8c0
 F test/count.test 454e1ce985c94d13efeac405ce54439f49336163
@@ -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 110998f18a7ad1ddaffab048cabef675d882cbb8
+P 716fccea58c4c217e68e04e0776e44ae39c11950
-R c2ae620b83a5c070a034b2845bc5bdf6
+R 1ee5a6818bfe745a7b3a3e753bd0011a
-U drh
+U danielk1977
-Z 3dae4d3e2687351a2da40f1b63712770
+Z 0104982a535ea93c433ad3cd1f268c89

manifest.uuid

@@ -1 +1 @@
-716fccea58c4c217e68e04e0776e44ae39c11950
+0b41dfc066b60ccabbf1a9ab4db41ebcb73a2799

src/btree.c

@@ -9,7 +9,7 @@
 **    May you share freely, never taking more than you give.
 **
 *************************************************************************
-** $Id: btree.c,v 1.696 2009/07/21 19:02:21 drh Exp $
+** $Id: btree.c,v 1.697 2009/07/21 19:25:24 danielk1977 Exp $
 **
 ** This file implements a external (disk-based) database using BTrees.
 ** See the header comment on "btreeInt.h" for additional information.
@@ -1540,9 +1540,12 @@ static Pgno pagerPagecount(BtShared *pBt){
 }
 
 /*
-** Get a page from the pager and initialize it.  This routine
+** Get a page from the pager and initialize it.  This routine is just a
-** is just a convenience wrapper around separate calls to
+** convenience wrapper around separate calls to btreeGetPage() and 
-** btreeGetPage() and btreeInitPage().
+** btreeInitPage().
+**
+** If an error occurs, then the value *ppPage is set to is undefined. It
+** may remain unchanged, or it may be set to an invalid value.
 */
 static int getAndInitPage(
   BtShared *pBt,          /* The database file */
@@ -1550,39 +1553,25 @@ static int getAndInitPage(
   MemPage **ppPage     /* Write the page pointer here */
 ){
   int rc;
-  MemPage *pPage;
+  TESTONLY( Pgno iLastPg = pagerPagecount(pBt); )
-
   assert( sqlite3_mutex_held(pBt->mutex) );
-  if( pgno==0 ){
+
-    return SQLITE_CORRUPT_BKPT; 
+  rc = btreeGetPage(pBt, pgno, ppPage, 0);
+  if( rc==SQLITE_OK ){
+    rc = btreeInitPage(*ppPage);
+    if( rc!=SQLITE_OK ){
+      releasePage(*ppPage);
+    }
   }
 
-  /* It is often the case that the page we want is already in cache.
+  /* If the requested page number was either 0 or greater than the page
-  ** If so, get it directly.  This saves us from having to call
+  ** number of the last page in the database, this function should return
-  ** pagerPagecount() to make sure pgno is within limits, which results
+  ** SQLITE_CORRUPT or some other error (i.e. SQLITE_FULL). Check that this
-  ** in a measureable performance improvements.
+  ** is the case.  */
-  */
+  assert( (pgno>0 && pgno<=iLastPg) || rc!=SQLITE_OK );
-  *ppPage = pPage = btreePageLookup(pBt, pgno);
+  testcase( pgno==0 );
-  if( pPage ){
+  testcase( pgno==iLastPg );
-    /* Page is already in cache */
+
-    rc = SQLITE_OK;
-  }else{
-    /* Page not in cache.  Acquire it. */
-    testcase( pgno==pagerPagecount(pBt) );
-    if( pgno>pagerPagecount(pBt) ){
-      return SQLITE_CORRUPT_BKPT; 
-    }
-    rc = btreeGetPage(pBt, pgno, ppPage, 0);
-    if( rc ) return rc;
-    pPage = *ppPage;
-  }
-  if( !pPage->isInit ){
-    rc = btreeInitPage(pPage);
-  }
-  if( rc!=SQLITE_OK ){
-    releasePage(pPage);
-    *ppPage = 0;
-  }
   return rc;
 }
 
@@ -2207,6 +2196,8 @@ static int lockBtree(BtShared *pBt){
 
   assert( sqlite3_mutex_held(pBt->mutex) );
   assert( pBt->pPage1==0 );
+  rc = sqlite3PagerSharedLock(pBt->pPager);
+  if( rc!=SQLITE_OK ) return rc;
   rc = btreeGetPage(pBt, 1, &pPage1, 0);
   if( rc!=SQLITE_OK ) return rc;
 
@@ -5705,7 +5696,7 @@ static int balance_nonroot(
   while( 1 ){
     rc = getAndInitPage(pBt, pgno, &apOld[i]);
     if( rc ){
-      memset(apOld, 0, i*sizeof(MemPage*));
+      memset(apOld, 0, (i+1)*sizeof(MemPage*));
       goto balance_cleanup;
     }
     nMaxCells += 1+apOld[i]->nCell+apOld[i]->nOverflow;

src/pager.c

@@ -18,7 +18,7 @@
 ** file simultaneously, or one process from reading the database while
 ** another is writing.
 **
-** @(#) $Id: pager.c,v 1.609 2009/07/18 20:01:37 drh Exp $
+** @(#) $Id: pager.c,v 1.610 2009/07/21 19:25:24 danielk1977 Exp $
 */
 #ifndef SQLITE_OMIT_DISKIO
 #include "sqliteInt.h"
@@ -1127,7 +1127,7 @@ static void pager_unlock(Pager *pPager){
     /* If the file is unlocked, somebody else might change it. The
     ** values stored in Pager.dbSize etc. might become invalid if
     ** this happens. TODO: Really, this doesn't need to be cleared
-    ** until the change-counter check fails in pagerSharedLock().
+    ** until the change-counter check fails in PagerSharedLock().
     */
     pPager->dbSizeValid = 0;
 
@@ -3545,10 +3545,12 @@ static int readDbPage(PgHdr *pPg){
 }
 
 /*
-** This function is called whenever the upper layer requests a database
+** This function is called to obtain a shared lock on the database file.
-** page is requested, before the cache is checked for a suitable page
+** It is illegal to call sqlite3PagerAcquire() until after this function
-** or any data is read from the database. It performs the following
+** has been successfully called. If a shared-lock is already held when
-** two functions:
+** this function is called, it is a no-op.
+**
+** The following operations are also performed by this function.
 **
 **   1) If the pager is currently in PAGER_UNLOCK state (no lock held
 **      on the database file), then an attempt is made to obtain a
@@ -3574,7 +3576,7 @@ static int readDbPage(PgHdr *pPg){
 ** IO error occurs while locking the database, checking for a hot-journal
 ** file or rolling back a journal file, the IO error code is returned.
 */
-static int pagerSharedLock(Pager *pPager){
+int sqlite3PagerSharedLock(Pager *pPager){
   int rc = SQLITE_OK;                /* Return code */
   int isErrorReset = 0;              /* True if recovering from error state */
 
@@ -3787,11 +3789,6 @@ static void pagerDropPage(DbPage *pPg){
 ** reference has type DbPage*). If the requested reference is 
 ** successfully obtained, it is copied to *ppPage and SQLITE_OK returned.
 **
-** This function calls pagerSharedLock() to obtain a SHARED lock on
-** the database file if such a lock or greater is not already held.
-** This may cause hot-journal rollback or a cache purge. See comments
-** above function pagerSharedLock() for details.
-**
 ** If the requested page is already in the cache, it is returned. 
 ** Otherwise, a new page object is allocated and populated with data
 ** read from the database file. In some cases, the pcache module may
@@ -3843,62 +3840,65 @@ int sqlite3PagerAcquire(
   DbPage **ppPage,    /* Write a pointer to the page here */
   int noContent       /* Do not bother reading content from disk if true */
 ){
-  PgHdr *pPg = 0;
   int rc;
+  PgHdr *pPg;
 
   assert( assert_pager_state(pPager) );
-  assert( pPager->state==PAGER_UNLOCK 
+  assert( pPager->state>PAGER_UNLOCK );
-       || sqlite3PcacheRefCount(pPager->pPCache)>0 
-       || pgno==1
-  );
 
-  /* The maximum page number is 2^31. Return SQLITE_CORRUPT if a page
+  if( pgno==0 ){
-  ** number greater than this, or zero, is requested.
-  */
-  if( pgno>PAGER_MAX_PGNO || pgno==0 || pgno==PAGER_MJ_PGNO(pPager) ){
     return SQLITE_CORRUPT_BKPT;
   }
 
-  /* Make sure we have not hit any critical errors.
+  /* If the pager is in the error state, return an error immediately. 
-  */ 
+  ** Otherwise, request the page from the PCache layer. */
-  assert( pPager!=0 );
+  if( pPager->errCode!=SQLITE_OK && pPager->errCode!=SQLITE_FULL ){
-  *ppPage = 0;
+    rc = pPager->errCode;
-
+  }else{
-  /* If this is the first page accessed, then get a SHARED lock
+    rc = sqlite3PcacheFetch(pPager->pPCache, pgno, 1, ppPage);
-  ** on the database file. pagerSharedLock() is a no-op if 
-  ** a database lock is already held.
-  */
-  rc = pagerSharedLock(pPager);
-  if( rc!=SQLITE_OK ){
-    return rc;
   }
-  assert( pPager->state!=PAGER_UNLOCK );
 
-  rc = sqlite3PcacheFetch(pPager->pPCache, pgno, 1, &pPg);
   if( rc!=SQLITE_OK ){
-    pagerUnlockIfUnused(pPager);
+    /* Either the call to sqlite3PcacheFetch() returned an error or the
-    return rc;
+    ** pager was already in the error-state when this function was called.
+    ** Set pPg to 0 and jump to the exception handler.  */
+    pPg = 0;
+    goto pager_acquire_err;
   }
-  assert( pPg->pgno==pgno );
+  assert( (*ppPage)->pgno==pgno );
-  assert( pPg->pPager==pPager || pPg->pPager==0 );
+  assert( (*ppPage)->pPager==pPager || (*ppPage)->pPager==0 );
-  if( pPg->pPager==0 ){
+
+  if( (*ppPage)->pPager ){
+    /* In this case the pcache already contains an initialized copy of
+    ** the page. Return without further ado.  */
+    PAGER_INCR(pPager->nHit);
+    return SQLITE_OK;
+
+  }else{
     /* The pager cache has created a new page. Its content needs to 
-    ** be initialized.
+    ** be initialized.  */
-    */
     int nMax;
+
     PAGER_INCR(pPager->nMiss);
+    pPg = *ppPage;
     pPg->pPager = pPager;
 
+    /* The maximum page number is 2^31. Return SQLITE_CORRUPT if a page
+    ** number greater than this, or the unused locking-page, is requested. */
+    if( pgno>PAGER_MAX_PGNO || pgno==PAGER_MJ_PGNO(pPager) ){
+      rc = SQLITE_CORRUPT_BKPT;
+      goto pager_acquire_err;
+    }
+
     rc = sqlite3PagerPagecount(pPager, &nMax);
     if( rc!=SQLITE_OK ){
-      sqlite3PagerUnref(pPg);
+      goto pager_acquire_err;
-      return rc;
     }
 
     if( nMax<(int)pgno || MEMDB || noContent ){
       if( pgno>pPager->mxPgno ){
-        sqlite3PagerUnref(pPg);
+	rc = SQLITE_FULL;
-        return SQLITE_FULL;
+	goto pager_acquire_err;
       }
       if( noContent ){
         /* Failure to set the bits in the InJournal bit-vectors is benign.
@@ -3924,19 +3924,23 @@ int sqlite3PagerAcquire(
       rc = readDbPage(pPg);
       if( rc!=SQLITE_OK ){
         pagerDropPage(pPg);
-        return rc;
+	pPg = 0;
+        goto pager_acquire_err;
       }
     }
 #ifdef SQLITE_CHECK_PAGES
     pPg->pageHash = pager_pagehash(pPg);
 #endif
-  }else{
-    /* The requested page is in the page cache. */
-    PAGER_INCR(pPager->nHit);
   }
 
-  *ppPage = pPg;
   return SQLITE_OK;
+
+pager_acquire_err:
+  assert( rc!=SQLITE_OK );
+  sqlite3PagerUnref(pPg);
+  pagerUnlockIfUnused(pPager);
+  *ppPage = 0;
+  return rc;
 }
 
 /*

src/pager.h

@@ -13,7 +13,7 @@
 ** subsystem.  The page cache subsystem reads and writes a file a page
 ** at a time and provides a journal for rollback.
 **
-** @(#) $Id: pager.h,v 1.102 2009/06/18 17:22:39 drh Exp $
+** @(#) $Id: pager.h,v 1.103 2009/07/21 19:25:24 danielk1977 Exp $
 */
 
 #ifndef _PAGER_H_
@@ -126,6 +126,7 @@ int sqlite3PagerCommitPhaseTwo(Pager*);
 int sqlite3PagerRollback(Pager*);
 int sqlite3PagerOpenSavepoint(Pager *pPager, int n);
 int sqlite3PagerSavepoint(Pager *pPager, int op, int iSavepoint);
+int sqlite3PagerSharedLock(Pager *pPager);
 
 /* Functions used to query pager state and configuration. */
 u8 sqlite3PagerIsreadonly(Pager*);

src/pcache.c

@@ -11,7 +11,7 @@
 *************************************************************************
 ** This file implements that page cache.
 **
-** @(#) $Id: pcache.c,v 1.45 2009/07/16 18:21:18 drh Exp $
+** @(#) $Id: pcache.c,v 1.46 2009/07/21 19:25:24 danielk1977 Exp $
 */
 #include "sqliteInt.h"
 
@@ -208,6 +208,7 @@ int sqlite3PcacheFetch(
   int eCreate;
 
   assert( pCache!=0 );
+  assert( createFlag==1 || createFlag==0 );
   assert( pgno>0 );
 
   /* If the pluggable cache (sqlite3_pcache*) has not been allocated,
@@ -225,10 +226,7 @@ int sqlite3PcacheFetch(
     pCache->pCache = p;
   }
 
-  eCreate = createFlag ? 1 : 0;
+  eCreate = createFlag * (1 + (!pCache->bPurgeable || !pCache->pDirty));
-  if( eCreate && (!pCache->bPurgeable || !pCache->pDirty) ){
-    eCreate = 2;
-  }
   if( pCache->pCache ){
     pPage = sqlite3GlobalConfig.pcache.xFetch(pCache->pCache, pgno, eCreate);
   }

src/test2.c

@@ -13,7 +13,7 @@
 ** is not included in the SQLite library.  It is used for automated
 ** testing of the SQLite library.
 **
-** $Id: test2.c,v 1.72 2009/07/16 18:21:18 drh Exp $
+** $Id: test2.c,v 1.73 2009/07/21 19:25:24 danielk1977 Exp $
 */
 #include "sqliteInt.h"
 #include "tcl.h"
@@ -342,7 +342,10 @@ static int page_get(
   }
   pPager = sqlite3TestTextToPtr(argv[1]);
   if( Tcl_GetInt(interp, argv[2], &pgno) ) return TCL_ERROR;
-  rc = sqlite3PagerGet(pPager, pgno, &pPage);
+  rc = sqlite3PagerSharedLock(pPager);
+  if( rc==SQLITE_OK ){
+    rc = sqlite3PagerGet(pPager, pgno, &pPage);
+  }
   if( rc!=SQLITE_OK ){
     Tcl_AppendResult(interp, errorName(rc), 0);
     return TCL_ERROR;

test/corruptB.test

@@ -20,7 +20,7 @@
 # contains a (corrupt) reference to a page greater than the configured
 # maximum page number.
 #
-# $Id: corruptB.test,v 1.3 2009/06/05 17:09:12 drh Exp $
+# $Id: corruptB.test,v 1.4 2009/07/21 19:25:24 danielk1977 Exp $
 
 set testdir [file dirname $argv0]
 source $testdir/tester.tcl
@@ -154,7 +154,7 @@ do_test corruptB-2.1.1 {
 do_test corruptB-2.1.2 {
   sqlite3 db test.db
   catchsql { SELECT * FROM t1 }
-} {1 {database disk image is malformed}}
+} {1 {database or disk is full}}
 
 #---------------------------------------------------------------------------