Skip to content

Commit

Permalink
[sql] Refactor to remove use of const_cast<>.
Browse files Browse the repository at this point in the history
The comments in these cases used incorrect reasoning for why the
const_cast<> was safe.  Whether the SQL statements modify the database
is irrelevant.  Add an internal helper to allow them to correctly
accomplish things in a const situation.

BUG=none
TEST=none


Review URL: https://chromiumcodereview.appspot.com/10736042

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@148031 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
shess@chromium.org committed Jul 24, 2012
1 parent 35bc8bf commit 2eec0a2
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 12 deletions.
36 changes: 25 additions & 11 deletions sql/connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ Connection::StatementRef::StatementRef()
stmt_(NULL) {
}

Connection::StatementRef::StatementRef(sqlite3_stmt* stmt)
: connection_(NULL),
stmt_(stmt) {
}

Connection::StatementRef::StatementRef(Connection* connection,
sqlite3_stmt* stmt)
: connection_(connection),
Expand Down Expand Up @@ -343,17 +348,32 @@ scoped_refptr<Connection::StatementRef> Connection::GetCachedStatement(
scoped_refptr<Connection::StatementRef> Connection::GetUniqueStatement(
const char* sql) {
if (!db_)
return new StatementRef(this, NULL); // Return inactive statement.
return new StatementRef(); // Return inactive statement.

sqlite3_stmt* stmt = NULL;
if (sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL) != SQLITE_OK) {
// This is evidence of a syntax error in the incoming SQL.
DLOG(FATAL) << "SQL compile error " << GetErrorMessage();
return new StatementRef(this, NULL);
return new StatementRef();
}
return new StatementRef(this, stmt);
}

scoped_refptr<Connection::StatementRef> Connection::GetUntrackedStatement(
const char* sql) const {
if (!db_)
return new StatementRef(); // Return inactive statement.

sqlite3_stmt* stmt = NULL;
int rc = sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL);
if (rc != SQLITE_OK) {
// This is evidence of a syntax error in the incoming SQL.
DLOG(FATAL) << "SQL compile error " << GetErrorMessage();
return new StatementRef();
}
return new StatementRef(stmt);
}

bool Connection::IsSQLValid(const char* sql) {
sqlite3_stmt* stmt = NULL;
if (sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL) != SQLITE_OK)
Expand All @@ -373,11 +393,8 @@ bool Connection::DoesIndexExist(const char* index_name) const {

bool Connection::DoesTableOrIndexExist(
const char* name, const char* type) const {
// GetUniqueStatement can't be const since statements may modify the
// database, but we know ours doesn't modify it, so the cast is safe.
Statement statement(const_cast<Connection*>(this)->GetUniqueStatement(
"SELECT name FROM sqlite_master "
"WHERE type=? AND name=?"));
const char* kSql = "SELECT name FROM sqlite_master WHERE type=? AND name=?";
Statement statement(GetUntrackedStatement(kSql));
statement.BindString(0, type);
statement.BindString(1, name);

Expand All @@ -390,10 +407,7 @@ bool Connection::DoesColumnExist(const char* table_name,
sql.append(table_name);
sql.append(")");

// Our SQL is non-mutating, so this cast is OK.
Statement statement(const_cast<Connection*>(this)->GetUniqueStatement(
sql.c_str()));

Statement statement(GetUntrackedStatement(sql.c_str()));
while (statement.Step()) {
if (!statement.ColumnString(1).compare(column_name))
return true;
Expand Down
11 changes: 10 additions & 1 deletion sql/connection.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Expand Down Expand Up @@ -338,6 +338,7 @@ class SQL_EXPORT Connection {
public:
// Default constructor initializes to an invalid statement.
StatementRef();
explicit StatementRef(sqlite3_stmt* stmt);
StatementRef(Connection* connection, sqlite3_stmt* stmt);

// When true, the statement can be used.
Expand Down Expand Up @@ -387,6 +388,14 @@ class SQL_EXPORT Connection {
bool ExecuteWithTimeout(const char* sql, base::TimeDelta ms_timeout)
WARN_UNUSED_RESULT;

// Internal helper for const functions. Like GetUniqueStatement(),
// except the statement is not entered into open_statements_,
// allowing this function to be const. Open statements can block
// closing the database, so only use in cases where the last ref is
// released before close could be called (which should always be the
// case for const functions).
scoped_refptr<StatementRef> GetUntrackedStatement(const char* sql) const;

// The actual sqlite database. Will be NULL before Init has been called or if
// Init resulted in an error.
sqlite3* db_;
Expand Down

0 comments on commit 2eec0a2

Please sign in to comment.