Nice job decoupling the 8-byte double serialization logic from relying on 64 bit integers.
Please consider creating an SQLite ticket for you patch so it might be incorporated into the official source tree. I must confess that I did not read the original 32-bit port thread URL in your first post - I just looked at the source code directly. Had I known that DRH had recommended the solution - I would not have bothered looking in the source for 64bit assumptions. But upon reading the link I was surprised when the person in the thread reported 32-bit porting success without apparently having to change any source code as you did. Maybe he worked with only small integers and did not use any double types. Or perhaps his compiler evaluated <<32 to zero for 32-bit types. --- Steffen Gutmann <[EMAIL PROTECTED]> wrote: > Joe, thanks for the prompt reply. the detailed answers and your > testing. I also had a closer look into the source and found some > suggestions for myself... > > First of all, I completely agree that defining an 'int' as > SQLITE_INT64_TYPE seems broken. It wasn't actually my idea but since > it was propsed by the author in a previous thread I thought I'll give > it a try. > > Also, I didn't understand the correlation with floating point types you > mentioned (SQLITE_OMIT_FLOATING_POINT=1) until I looked into vdbeaux.c > and saw the problem. Internally, sqlite takes the address of a double > as a pointer to an int64 and then reads/writes it as such from/into > memory. This part of sqlite is a bit 'hacky' as it makes certain > assumption about the size and layout of double versus int64. > > The issue of undefined results when shifting 32 bits left was new to me > (I would have expected the result to be zero). Thanks for pointing it > out and for the small test example that clearly shows the problem. > > After changing some pieces of the source code, I was able to arrive at > a version that passes all relevant test cases of quick.test. It fails > on 60 of the 27021 tests but manual inspection reveals that all these > cases deal with integer numbers out of the 32bit range or with other > integer overflow issues. I attach a pach of my changes in case someone > is interested. They still need verification though, as e.g. I only > tested them on a little-endian architecture. > > In my understanding floating points should be independent from integer > types, e.g. I would still like to see support for 32bit integers and > 64bit double values. The patch also fixes this. > > The changes include: > > util.c; add check for sizeof(int64) before shifting 32 bits > vdbeaux.c: add check for sizeof(int64) before shifting 32 bits > rewrote reading/writing of double type independent of int64 > (introduced hanndling of litte-endian/big-endian cases) > test[13].c: cast to (long long int) in printf with "%lld" format > use Tcl_WideInt when reading from a Tcl object > > That's all. I believe some of the changes would also be useful in > general. Let me know what you think. > > What would also be nice to see is if the tests requiring 64bit integers > are marked as such (or moved into a new test file). But this would be > a lot of work I guess... > > Best regards, > > Steffen > > > > ____________________________________________________________________________________ > Do you Yahoo!? > Everyone is raving about the all-new Yahoo! Mail beta. > http://new.mail.yahoo.com> Only in sqlite-3.3.8-int32: build > diff -ru sqlite-3.3.8/src/test1.c sqlite-3.3.8-int32/src/test1.c > --- sqlite-3.3.8/src/test1.c 2006-09-15 16:28:51.000000000 +0900 > +++ sqlite-3.3.8-int32/src/test1.c 2006-11-27 13:17:19.000000000 +0900 > @@ -342,7 +342,7 @@ > return TCL_ERROR; > } > if( getDbPointer(interp, argv[1], &db) ) return TCL_ERROR; > - sprintf(zBuf, "%lld", sqlite3_last_insert_rowid(db)); > + sprintf(zBuf, "%lld", (long long int)sqlite3_last_insert_rowid(db)); > Tcl_AppendResult(interp, zBuf, 0); > return SQLITE_OK; > } > @@ -1998,6 +1998,7 @@ > ){ > sqlite3_stmt *pStmt; > int idx; > + Tcl_WideInt tclValue; > i64 value; > int rc; > > @@ -2009,7 +2010,8 @@ > > if( getStmtPointer(interp, Tcl_GetString(objv[1]), &pStmt) ) return > TCL_ERROR; > if( Tcl_GetIntFromObj(interp, objv[2], &idx) ) return TCL_ERROR; > - if( Tcl_GetWideIntFromObj(interp, objv[3], &value) ) return TCL_ERROR; > + if( Tcl_GetWideIntFromObj(interp, objv[3], &tclValue) ) return TCL_ERROR; > + value = tclValue; > > rc = sqlite3_bind_int64(pStmt, idx, value); > if( sqlite3TestErrCode(interp, StmtToDb(pStmt), rc) ) return TCL_ERROR; > diff -ru sqlite-3.3.8/src/test3.c sqlite-3.3.8-int32/src/test3.c > --- sqlite-3.3.8/src/test3.c 2006-09-14 22:31:18.000000000 +0900 > +++ sqlite-3.3.8-int32/src/test3.c 2006-11-27 13:20:26.000000000 +0900 > @@ -769,10 +769,12 @@ > } > pCur = sqlite3TextToPtr(Tcl_GetString(objv[1])); > if( sqlite3BtreeFlags(pCur) & BTREE_INTKEY ){ > + Tcl_WideInt tclValue; > i64 iKey; > int len; > unsigned char *pBuf; > - if( Tcl_GetWideIntFromObj(interp, objv[2], &iKey) ) return TCL_ERROR; > + if( Tcl_GetWideIntFromObj(interp, objv[2], &tclValue) ) return TCL_ERROR; > + iKey = tclValue; > pBuf = Tcl_GetByteArrayFromObj(objv[3], &len); > rc = sqlite3BtreeInsert(pCur, 0, iKey, pBuf, len); > }else{ > @@ -1291,7 +1293,8 @@ > return TCL_ERROR; > } > if( in!=out ){ > - sprintf(zErr, "Wrote 0x%016llx and got back 0x%016llx", in, out); > + sprintf(zErr, "Wrote 0x%016llx and got back 0x%016llx", > + (long long int)in, (long long int)out); > Tcl_AppendResult(interp, zErr, 0); > return TCL_ERROR; > } > @@ -1307,7 +1310,7 @@ > } > if( in!=out ){ > sprintf(zErr, "Wrote 0x%016llx and got back 0x%016llx from > GetVarint32", > - in, out); > + (long long int)in, (long long int)out); > Tcl_AppendResult(interp, zErr, 0); > return TCL_ERROR; > } > diff -ru sqlite-3.3.8/src/util.c sqlite-3.3.8-int32/src/util.c > --- sqlite-3.3.8/src/util.c 2006-09-15 16:28:51.000000000 +0900 > +++ sqlite-3.3.8-int32/src/util.c 2006-11-27 13:59:48.000000000 +0900 > @@ -1223,7 +1223,7 @@ > int sqlite3PutVarint(unsigned char *p, u64 v){ > int i, j, n; > u8 buf[10]; > - if( v & (((u64)0xff000000)<<32) ){ > + if(sizeof(v) > 7 && v & (((u64)0xff000000)<<32) ){ > p[8] = v; > v >>= 8; > for(i=7; i>=0; i--){ > diff -ru sqlite-3.3.8/src/vdbeaux.c sqlite-3.3.8-int32/src/vdbeaux.c > --- sqlite-3.3.8/src/vdbeaux.c 2006-09-24 06:24:12.000000000 +0900 > +++ sqlite-3.3.8-int32/src/vdbeaux.c 2006-11-27 15:15:10.000000000 +0900 > @@ -1677,7 +1677,7 @@ > if( u<=32767 ) return 2; > if( u<=8388607 ) return 3; > if( u<=2147483647 ) return 4; > - if( u<=MAX_6BYTE ) return 5; > + if( sizeof(u)<6 || u<=MAX_6BYTE ) return 5; > return 6; > } > if( flags&MEM_Real ){ > @@ -1715,15 +1715,11 @@ > u32 serial_type = sqlite3VdbeSerialType(pMem, file_format); > int len; > > - /* Integer and Real */ > - if( serial_type<=7 && serial_type>0 ){ > + /* Integer */ > + if( serial_type<7 && serial_type>0 ){ > u64 v; > int i; > - if( serial_type==7 ){ > - v = *(u64*)&pMem->r; > - }else{ > - v = *(u64*)&pMem->i; > - } > + v = *(u64*)&pMem->i; > len = i = sqlite3VdbeSerialTypeLen(serial_type); > while( i-- ){ > buf[i] = (v&0xFF); > @@ -1732,6 +1728,32 @@ > return len; > } > > + /* Real */ > + if( serial_type==7 ){ > + const double x = 1.0; > + const char endian = *(const char *)&x; > + const char *t = (const char *)&pMem->r; > + const char *tend = t + sizeof(double); > + int i; > + > + len = i = sqlite3VdbeSerialTypeLen(serial_type); > + assert(len >= sizeof(double)); > + if (endian == 0) { /* little endian? */ > + while (i && t < tend) { > + buf[--i] = *t++; > + } > + } else { > + assert(endian == 0x3f); > + while (i && t < tend) { > + buf[--i] = *--tend; > + } > + } > + while( i-- ){ > + buf[i] = 0; /* pad unused size with 0 bytes */ > + } > + return len; > + } > + > /* String or blob */ > if( serial_type>=12 ){ > len = sqlite3VdbeSerialTypeLen(serial_type); > @@ -1782,33 +1804,58 @@ > case 5: { /* 6-byte signed integer */ > u64 x = (((signed char)buf[0])<<8) | buf[1]; > u32 y = (buf[2]<<24) | (buf[3]<<16) | (buf[4]<<8) | buf[5]; > - x = (x<<32) | y; > + if (sizeof(x) > 4) { > + x = (x<<32) | y; > + } else { > + if (x) { > + fprintf(stderr, "serial: ignoring upper 4 bytes (%x)\n", x); > + } > + x = y; > + } > pMem->i = *(i64*)&x; > pMem->flags = MEM_Int; > return 6; > } > - case 6: /* 8-byte signed integer */ > - case 7: { /* IEEE floating point */ > + case 6: { /* 8-byte signed integer */ > u64 x; > u32 y; > -#if !defined(NDEBUG) && !defined(SQLITE_OMIT_FLOATING_POINT) > - /* Verify that integers and floating point values use the same > - ** byte order. The byte order differs on some (broken) architectures. > - */ > - static const u64 t1 = ((u64)0x3ff00000)<<32; > - assert( 1.0==*(double*)&t1 ); > -#endif > > x = (buf[0]<<24) | (buf[1]<<16) | (buf[2]<<8) | buf[3]; > y = (buf[4]<<24) | (buf[5]<<16) | (buf[6]<<8) | buf[7]; > - x = (x<<32) | y; > - if( serial_type==6 ){ > - pMem->i = *(i64*)&x; > - pMem->flags = MEM_Int; > - }else{ > - pMem->r = *(double*)&x; > - pMem->flags = MEM_Real; > + if (sizeof(x) > 4) { > + x = (x<<32) | y; > + } else { > + if (x) { > + fprintf(stderr, "serial: ignoring upper 4 bytes (%x)\n", x); > + } > + x = y; > } > + pMem->i = *(i64*)&x; > + pMem->flags = MEM_Int; > + return 8; > + } > + case 7: { /* IEEE floating point */ > + const double x = 1.0; > + const char endian = *(const char *)&x; > + char *t = (char *)&pMem->r; > + char *tend = t + sizeof(double); > + int i; > + > === message truncated ===> ----------------------------------------------------------------------------- > To unsubscribe, send email to [EMAIL PROTECTED] > ----------------------------------------------------------------------------- ____________________________________________________________________________________ Cheap talk? Check out Yahoo! Messenger's low PC-to-Phone call rates. http://voice.yahoo.com ----------------------------------------------------------------------------- To unsubscribe, send email to [EMAIL PROTECTED] -----------------------------------------------------------------------------
