Skip to content

Commit

Permalink
sql: Switch some Database APIs from const char* to StringPiece.
Browse files Browse the repository at this point in the history
StringPiece is safer than a raw pointer. The raw pointer is only needed
where the underlying SQLite API uses a C-style null-terminated string.

Change-Id: Ia4900ba94e5f40bc91be3eee50029fe87cd2eba6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3019892
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900742}
  • Loading branch information
pwnall authored and Chromium LUCI CQ committed Jul 13, 2021
1 parent 903c962 commit 83d940d
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 17 deletions.
24 changes: 13 additions & 11 deletions sql/database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "base/single_thread_task_runner.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_piece.h"
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
Expand Down Expand Up @@ -111,11 +112,10 @@ int BackupDatabase(sqlite3* src, sqlite3* dst, const char* db_name) {
// Be very strict on attachment point. SQLite can handle a much wider
// character set with appropriate quoting, but Chromium code should
// just use clean names to start with.
bool ValidAttachmentPoint(const char* attachment_point) {
for (size_t i = 0; attachment_point[i]; ++i) {
if (!(base::IsAsciiDigit(attachment_point[i]) ||
base::IsAsciiAlpha(attachment_point[i]) ||
attachment_point[i] == '_')) {
bool ValidAttachmentPoint(base::StringPiece attachment_point) {
for (char attachment_char : attachment_point) {
if (!(base::IsAsciiDigit(attachment_char) ||
base::IsAsciiAlpha(attachment_char) || attachment_char == '_')) {
return false;
}
}
Expand Down Expand Up @@ -1071,7 +1071,7 @@ void Database::RollbackAllTransactions() {
}

bool Database::AttachDatabase(const base::FilePath& other_db_path,
const char* attachment_point,
base::StringPiece attachment_point,
InternalApiToken) {
TRACE_EVENT0("sql", "Database::AttachDatabase");

Expand All @@ -1089,7 +1089,8 @@ bool Database::AttachDatabase(const base::FilePath& other_db_path,
return s.Run();
}

bool Database::DetachDatabase(const char* attachment_point, InternalApiToken) {
bool Database::DetachDatabase(base::StringPiece attachment_point,
InternalApiToken) {
TRACE_EVENT0("sql", "Database::DetachDatabase");

DCHECK(ValidAttachmentPoint(attachment_point));
Expand Down Expand Up @@ -1322,19 +1323,20 @@ bool Database::IsSQLValid(const char* sql) {
return true;
}

bool Database::DoesIndexExist(const char* index_name) const {
bool Database::DoesIndexExist(base::StringPiece index_name) const {
return DoesSchemaItemExist(index_name, "index");
}

bool Database::DoesTableExist(const char* table_name) const {
bool Database::DoesTableExist(base::StringPiece table_name) const {
return DoesSchemaItemExist(table_name, "table");
}

bool Database::DoesViewExist(const char* view_name) const {
bool Database::DoesViewExist(base::StringPiece view_name) const {
return DoesSchemaItemExist(view_name, "view");
}

bool Database::DoesSchemaItemExist(const char* name, const char* type) const {
bool Database::DoesSchemaItemExist(base::StringPiece name,
base::StringPiece type) const {
static const char kSql[] =
"SELECT 1 FROM sqlite_master WHERE type=? AND name=?";
Statement statement(GetUntrackedStatement(kSql));
Expand Down
14 changes: 8 additions & 6 deletions sql/database.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "base/gtest_prod_util.h"
#include "base/memory/ref_counted.h"
#include "base/sequence_checker.h"
#include "base/strings/string_piece.h"
#include "base/threading/scoped_blocking_call.h"
#include "sql/internal_api_token.h"
#include "sql/sql_features.h"
Expand Down Expand Up @@ -323,9 +324,9 @@ class COMPONENT_EXPORT(SQL) Database {
// These APIs are only exposed for use in recovery. They are extremely subtle
// and are not useful for features built on top of //sql.
bool AttachDatabase(const base::FilePath& other_db_path,
const char* attachment_point,
base::StringPiece attachment_point,
InternalApiToken);
bool DetachDatabase(const char* attachment_point, InternalApiToken);
bool DetachDatabase(base::StringPiece attachment_point, InternalApiToken);

// Statements ----------------------------------------------------------------

Expand Down Expand Up @@ -396,9 +397,9 @@ class COMPONENT_EXPORT(SQL) Database {
// Returns true if the given structure exists. Instead of test-then-create,
// callers should almost always prefer the "IF NOT EXISTS" version of the
// CREATE statement.
bool DoesIndexExist(const char* index_name) const;
bool DoesTableExist(const char* table_name) const;
bool DoesViewExist(const char* table_name) const;
bool DoesIndexExist(base::StringPiece index_name) const;
bool DoesTableExist(base::StringPiece table_name) const;
bool DoesViewExist(base::StringPiece table_name) const;

// Returns true if a column with the given name exists in the given table.
//
Expand Down Expand Up @@ -529,7 +530,8 @@ class COMPONENT_EXPORT(SQL) Database {
}

// Internal helper for Does*Exist() functions.
bool DoesSchemaItemExist(const char* name, const char* type) const;
bool DoesSchemaItemExist(base::StringPiece name,
base::StringPiece type) const;

// Accessors for global error-expecter, for injecting behavior during tests.
// See test/scoped_error_expecter.h.
Expand Down

0 comments on commit 83d940d

Please sign in to comment.