Skip to content

Commit

Permalink
sql: Remove redundant PRAGMA secure_delete=ON.
Browse files Browse the repository at this point in the history
Chrome builds SQLite with SQLITE_SECURE_DELETE [1], so the default value
for the secure_delete PRAGMA [2] is already true.

[1] https://www.sqlite.org/compile.html#secure_delete
[2] https://www.sqlite.org/pragma.html#pragma_secure_delete

Bug: 910955
Change-Id: I27ef32770a980c8c6030675e79698d7a31e47a9d
Reviewed-on: https://chromium-review.googlesource.com/c/1357824
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613866}
  • Loading branch information
pwnall authored and Commit Bot committed Dec 5, 2018
1 parent f13f08a commit f1e9443
Show file tree
Hide file tree
Showing 5 changed files with 680 additions and 688 deletions.
13 changes: 4 additions & 9 deletions sql/database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1663,8 +1663,8 @@ bool Database::OpenInternal(const std::string& file_name,
OnSqliteError(err, nullptr, "PRAGMA auto_vacuum");

// Retry or bail out if the error handler poisoned the handle.
// TODO(shess): Move this handling to one place (see also sqlite3_open and
// secure_delete). Possibly a wrapper function?
// TODO(shess): Move this handling to one place (see also sqlite3_open).
// Possibly a wrapper function?
if (poisoned_) {
Close();
if (retry_flag == RETRY_ON_POISON)
Expand Down Expand Up @@ -1715,13 +1715,8 @@ bool Database::OpenInternal(const std::string& file_name,
ignore_result(ExecuteWithTimeout(cache_size_sql.c_str(), kBusyTimeout));
}

if (!ExecuteWithTimeout("PRAGMA secure_delete=ON", kBusyTimeout)) {
bool was_poisoned = poisoned_;
Close();
if (was_poisoned && retry_flag == RETRY_ON_POISON)
return OpenInternal(file_name, NO_RETRY);
return false;
}
static_assert(SQLITE_SECURE_DELETE == 1,
"Chrome assumes secure_delete is on by default.");

// Set a reasonable chunk size for larger files. This reduces churn from
// remapping memory on size changes. It also reduces filesystem
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ SQLITE_TRANSACTION statement failed: could not prepare statement (23 not authori
SQLITE_ATTACH statement failed: could not prepare statement (23 not authorized)
SQLITE_DETACH statement failed: could not prepare statement (23 not authorized)
SQLITE_REINDEX statement failed: could not prepare statement (1 near "REINDEX": syntax error)
SQLITE_ANALYZE statement failed: could not prepare statement (23 not authorized)
SQLITE_ANALYZE statement failed: could not prepare statement (1 near "ANALYZE": syntax error)
SQLITE_DROP_INDEX statement succeeded.
SQLITE_DROP_TEMP_TABLE statement succeeded.
SQLITE_DROP_TEMP_TRIGGER statement succeeded.
Expand Down Expand Up @@ -61,7 +61,7 @@ SQLITE_TRANSACTION statement failed: could not prepare statement (23 not authori
SQLITE_ATTACH statement failed: could not prepare statement (23 not authorized)
SQLITE_DETACH statement failed: could not prepare statement (23 not authorized)
SQLITE_REINDEX statement failed: could not prepare statement (1 near "REINDEX": syntax error)
SQLITE_ANALYZE statement failed: could not prepare statement (23 not authorized)
SQLITE_ANALYZE statement failed: could not prepare statement (1 near "ANALYZE": syntax error)
SQLITE_DROP_INDEX statement failed: could not prepare statement (23 not authorized)
SQLITE_DROP_TEMP_TABLE statement failed: could not prepare statement (23 not authorized)
SQLITE_DROP_TEMP_TRIGGER statement failed: could not prepare statement (23 not authorized)
Expand Down
11 changes: 11 additions & 0 deletions third_party/sqlite/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,17 @@ config("chromium_sqlite3_compile_options") {
"SQLITE_OMIT_SHARED_CACHE",
"SQLITE_USE_ALLOCA",

# Chrome doesn't use the ANALYZE SQLite extension.
#
# ANALYZE [1] is non-standard, and currently performs a table scan to
# update statistics used by the query planner. Chrome uses straightforward
# database schemas which do not require the level of fine tuning provided
# by ANALYZE, and we generally cannot afford the I/O cost of the required
# table scans.
#
# [1] https://www.sqlite.org/lang_analyze.html
"SQLITE_OMIT_ANALYZE",

# Chrome initializes SQLite manually in //sql/connection.cc.
"SQLITE_OMIT_AUTOINIT",

Expand Down
Loading

0 comments on commit f1e9443

Please sign in to comment.