Re: [sqlite] Potential corruption bug in 2.8.17. Patch attached.

2006-10-24 Thread drh 
[EMAIL PROTECTED] wrote:
 This was likely a typo.  In its current state, it's accessing uninitialized
 memory.  It looks like it's conceivable that an incorrect nextRowid could be
 later used if the uninitialized value happens to be a small integer (smaller
 than pC-nextRowid) and the valid flag therefore doesn't get set to false.
 
 --- vdbe.c~   2005-12-19 12:42:25.0 -0500
 +++ vdbe.c2006-10-22 16:32:45.0 -0400
 @@ -2937,7 +2937,7 @@
if( pOp-p2  OPFLAG_NCHANGE ) db-nChange++;
if( pOp-p2  OPFLAG_LASTROWID ) db-lastRowid = pNos-i;
if( pOp-p2  OPFLAG_CSCHANGE ) db-csChange++;
 -  if( pC-nextRowidValid  pTos-i=pC-nextRowid ){
 +  if( pC-nextRowidValid  pNos-i=pC-nextRowid ){
  pC-nextRowidValid = 0;
}
  }
 

As it happens, pC-nextRowidValid is always false in this
context, as far as I can tell, so the pTos-i variable is
never accessed.  The fix checked in was to remove the test
altogether and unconditionally set pC-nextRowidValid to 0.

Because the code is unreachable, this fix does not require
a new release of SQLite 2.8.

--
D. Richard Hipp  [EMAIL PROTECTED]


-
To unsubscribe, send email to [EMAIL PROTECTED]
-

Re: [sqlite] Potential corruption bug in 2.8.17. Patch attached.

2006-10-24 Thread Derrell . Lipman 
[EMAIL PROTECTED] writes:

 [EMAIL PROTECTED] wrote:
 This was likely a typo.  In its current state, it's accessing uninitialized
 memory.  It looks like it's conceivable that an incorrect nextRowid could be
 later used if the uninitialized value happens to be a small integer (smaller
 than pC-nextRowid) and the valid flag therefore doesn't get set to false.
 
 --- vdbe.c~  2005-12-19 12:42:25.0 -0500
 +++ vdbe.c   2006-10-22 16:32:45.0 -0400
 @@ -2937,7 +2937,7 @@
if( pOp-p2  OPFLAG_NCHANGE ) db-nChange++;
if( pOp-p2  OPFLAG_LASTROWID ) db-lastRowid = pNos-i;
if( pOp-p2  OPFLAG_CSCHANGE ) db-csChange++;
 -  if( pC-nextRowidValid  pTos-i=pC-nextRowid ){
 +  if( pC-nextRowidValid  pNos-i=pC-nextRowid ){
  pC-nextRowidValid = 0;
}
  }
 

 As it happens, pC-nextRowidValid is always false in this
 context, as far as I can tell, so the pTos-i variable is
 never accessed.

That explains why in many years of use, I've never seen an actual corruption
caused by this.  Thanks.

 The fix checked in was to remove the test altogether and unconditionally set
 pC-nextRowidValid to 0.

If, as you say, pC-nextRowidValid is always false anyway, wouldn't the
correct fix be to not even unconditionally set pC-nextRowidValid to 0; just
delete those two original lines entirely?  It sounds like you're now
unnecessarily setting a variable that's already false to false. (Or did I
misunderstand your statement?)

Derrell

-
To unsubscribe, send email to [EMAIL PROTECTED]
-

Re: [sqlite] Potential corruption bug in 2.8.17. Patch attached.

2006-10-24 Thread drh 
[EMAIL PROTECTED] wrote:
  
  --- vdbe.c~2005-12-19 12:42:25.0 -0500
  +++ vdbe.c 2006-10-22 16:32:45.0 -0400
  @@ -2937,7 +2937,7 @@
 if( pOp-p2  OPFLAG_NCHANGE ) db-nChange++;
 if( pOp-p2  OPFLAG_LASTROWID ) db-lastRowid = pNos-i;
 if( pOp-p2  OPFLAG_CSCHANGE ) db-csChange++;
  -  if( pC-nextRowidValid  pTos-i=pC-nextRowid ){
  +  if( pC-nextRowidValid  pNos-i=pC-nextRowid ){
   pC-nextRowidValid = 0;
 }
   }
  
 
 
  The fix checked in was to remove the test altogether and unconditionally set
  pC-nextRowidValid to 0.
 
 If, as you say, pC-nextRowidValid is always false anyway, wouldn't the
 correct fix be to not even unconditionally set pC-nextRowidValid to 0; just
 delete those two original lines entirely?  It sounds like you're now
 unnecessarily setting a variable that's already false to false. (Or did I
 misunderstand your statement?)
 

I could not find a case where pC-nextRowidValid was true.  But
neither did I prove it was impossible.  Lets just say it was
very unlikely.  

Setting pC-nextRowidValid merely invalidates a cache.  Invalidating
the cache might make the code a little slower, but it will still get
the correct answer.  So it is always safe to set pC-newRowidValid to
0.  But we cannot leave pC-nextRowidValid in the true state because
that might leave an invalid value in the cache, resulting in a wrong
answer.  So for safety, we always clear the cache here, even though
we have never seen an example where it is necessary.  Better safe than
sorry.
--
D. Richard Hipp  [EMAIL PROTECTED]


-
To unsubscribe, send email to [EMAIL PROTECTED]
-

[sqlite] Potential corruption bug in 2.8.17. Patch attached.

2006-10-22 Thread Derrell . Lipman 
This was likely a typo.  In its current state, it's accessing uninitialized
memory.  It looks like it's conceivable that an incorrect nextRowid could be
later used if the uninitialized value happens to be a small integer (smaller
than pC-nextRowid) and the valid flag therefore doesn't get set to false.

--- vdbe.c~ 2005-12-19 12:42:25.0 -0500
+++ vdbe.c  2006-10-22 16:32:45.0 -0400
@@ -2937,7 +2937,7 @@
   if( pOp-p2  OPFLAG_NCHANGE ) db-nChange++;
   if( pOp-p2  OPFLAG_LASTROWID ) db-lastRowid = pNos-i;
   if( pOp-p2  OPFLAG_CSCHANGE ) db-csChange++;
-  if( pC-nextRowidValid  pTos-i=pC-nextRowid ){
+  if( pC-nextRowidValid  pNos-i=pC-nextRowid ){
 pC-nextRowidValid = 0;
   }
 }

Cheers,

Derrell

-
To unsubscribe, send email to [EMAIL PROTECTED]
-