1

I am looking at some vendor code and there is a query like this:

BOOL BindQuery(sqlite3_stmt* stmt, PARAMS* params)
{
    char temp[150] = "";
    char paramBuf[10] = "";
    if (currStmt == NULL) return FALSE;

    sprintf(paramBuf, "%d", (int)params->someParam);
    strcpy(temp, "%");
    strcat(temp, tempVolt);
    strcat(temp, "%");
    sqlite3_bind_text(stmt, 4, temp, strlen(temp), SQLITE_STATIC);
    return TRUE;
}

Later down the road that query get executed. The problem is that this query never matches, even though it should.

I believe the problem is that sqlite3_bind_text binds a local variable and SQLite keep the pointer to the original local variable. So when it goes out of scope, it may have already been overwritten. The fix seems to be to use SQLITE_TRANSIENT instead. Can anyone confirm my thinking? Or am I off-base?

Another curious issue is that the vendor was never able to reproduce it. Luck?

jn1kk
  • 5,012
  • 2
  • 45
  • 72
  • 2
    "If the fifth argument has the value SQLITE_TRANSIENT, then SQLite makes its own private copy of the data immediately, before the sqlite3_bind_*() routine returns." Sounds like you're correct. – Jay May 21 '15 at 14:46

2 Answers2

1

Yes, this code is wrong. The documentation says:

If the fifth argument is the special value SQLITE_STATIC, then SQLite assumes that the information is in static, unmanaged space

but that local variable is not static.

This code might work if that part of the stack happens to avoid being overwritten until the query is executed.

CL.
  • 173,858
  • 17
  • 217
  • 259
0

If someone is creating a modern C++ wrapper over sqlite C ABI, then we can use an R-Value Reference to selectively use the SQL_TRANSIENT for temporary objects passed.

Something like below

class Statement
{
/*Other Logic*/
/* Other Bind() overloads using SQLITE_STATIC */
void Bind(const int index, std::string && text) const
{
  if(SQLITE_OK != sqlite3_bind_text(FetchABI(),index,text.c_str(),text.size(),SQLITE_TRANSIENT))
{
 // Error Handling
}
}
/* Bind overload for std::wstring */
};

When we pass a temporary object the compiler is smart enough to pick the right overload and hence we avoid the cost of SQLite making private copies each and everywhere( this might not be required for small sized apps )

Inside main()

Statement obj;
obj.Prepare(cx,"Select ?1"); // cx is the connection not defined here for brevity
obj.Bind(1,std::string("This value is copied"));
// Or the second overload
obj.Bind(1,std::wstring(L"This too is copied"));

Note: FetchABI() gets the underlying handle, the implementation of which is according a someone's whim and fancies.

sjsam
  • 21,411
  • 5
  • 55
  • 102