sql: Document raw pointer usage and remove unused member in recovery.

This addresses post-submit feedback on https://crrev.com/c/1546942.

Bug: 945204
Change-Id: I1106d820a1fff20eb8bab62baf474669e3a03ec8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613966
Auto-Submit: Victor Costan <pwnall@chromium.org>
Commit-Queue: Darwin Huang <huangdarwin@chromium.org>
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660195}

sql/recover_module/btree.h

@@ -70,6 +70,9 @@ class InnerPageDecoder {
   // The number of the B-tree page this reader is reading.
   const int page_id_;
   // Used to read the tree page.
+  //
+  // Raw pointer usage is acceptable because this instance's owner is expected
+  // to ensure that the DatabasePageReader outlives this.
   DatabasePageReader* const db_reader_;
   // Caches the ComputeCellCount() value for this reader's page.
   const int cell_count_ = ComputeCellCount(db_reader_);
@@ -162,6 +165,9 @@ class LeafPageDecoder {
   // The number of the B-tree page this reader is reading.
   const int64_t page_id_;
   // Used to read the tree page.
+  //
+  // Raw pointer usage is acceptable because this instance's owner is expected
+  // to ensure that the DatabasePageReader outlives this.
   DatabasePageReader* const db_reader_;
   // Caches the ComputeCellCount() value for this reader's page.
   const int cell_count_ = ComputeCellCount(db_reader_);

sql/recover_module/cursor.h

@@ -107,6 +107,10 @@ class VirtualCursor {
   sqlite3_vtab_cursor sqlite_cursor_;
 
   // The table this cursor was created for.
+  //
+  // Raw pointer usage is acceptable because SQLite will ensure that the
+  // VirtualTable, which is passed around as a sqlite3_vtab*, will outlive this
+  // cursor, which is passed around as a sqlite3_cursor*.
   VirtualTable* const table_;
 
   // Reads database pages for this cursor.
@@ -133,4 +137,4 @@ class VirtualCursor {
 }  // namespace recover
 }  // namespace sql
 
-#endif  // SQL_RECOVER_MODULE_CURSOR_H_
+#endif  // SQL_RECOVER_MODULE_CURSOR_H_

sql/recover_module/pager.h

@@ -135,9 +135,11 @@ class DatabasePageReader {
   // Points to the last page successfully read by ReadPage().
   // Set to kInvalidPageId if the last read was unsuccessful.
   int page_id_ = kInvalidPageId;
-  //
+  // Stores the bytes of the last page successfully read by ReadPage().
+  // The content is undefined if the last call to ReadPage() did not succeed.
   const std::unique_ptr<uint8_t[]> page_data_;
-  // Raw
+  // Raw pointer usage is acceptable because this instance's owner is expected
+  // to ensure that the VirtualTable outlives this.
   VirtualTable* const table_;
   int page_size_ = 0;
 
@@ -147,4 +149,4 @@ class DatabasePageReader {
 }  // namespace recover
 }  // namespace sql
 
-#endif  // SQL_RECOVER_MODULE_PAGER_H_
+#endif  // SQL_RECOVER_MODULE_PAGER_H_

sql/recover_module/payload.h

@@ -101,6 +101,9 @@ class LeafPayloadReader {
   bool PopulateNextOverflowPageId();
 
   // Used to read the pages containing the payload.
+  //
+  // Raw pointer usage is acceptable because this instance's owner is expected
+  // to ensure that the DatabasePageReader outlives this.
   DatabasePageReader* const db_reader_;
 
   // Total size of the current payload.

sql/recover_module/record.h

@@ -65,6 +65,14 @@ class RecordReader {
     bool has_inline_value;
   };
 
+  // Creates an uninitialized record reader from a SQLite table B-tree.
+  //
+  // |payload_reader_| must outlive this instance, and should always point to
+  // leaf pages in the same tree. |column_count| must match the number of
+  // columns in the table's schema.
+  //
+  // The underlying table should not be modified while the record is
+  // initialized.
   explicit RecordReader(LeafPayloadReader* payload_reader_, int column_count);
   ~RecordReader();
 
@@ -128,6 +136,9 @@ class RecordReader {
   std::vector<uint8_t> header_buffer_;
 
   // Brings the record's bytes from the SQLite database pages.
+  //
+  // Raw pointer usage is acceptable because this instance's owner is expected
+  // to ensure that the LeafPayloadReader outlives this.
   LeafPayloadReader* const payload_reader_;
 
   // The number of columns in the table schema. No payload should have more than

sql/recover_module/table.cc

@@ -167,8 +167,7 @@ VirtualTable::VirtualTable(sqlite3* sqlite_db,
                            int root_page_id,
                            int page_size,
                            std::vector<RecoveredColumnSpec> column_specs)
-    : sqlite_db_(sqlite_db),
-      sqlite_file_(sqlite_file),
+    : sqlite_file_(sqlite_file),
       root_page_id_(root_page_id),
       page_size_(page_size),
       column_specs_(std::move(column_specs)) {
@@ -205,8 +204,6 @@ VirtualCursor* VirtualTable::CreateCursor() {
   open_cursor_count_.fetch_add(1, std::memory_order_relaxed);
 #endif  // DCHECK_IS_ON()
 
-  (void)sqlite_db_;
-
   VirtualCursor* result = new VirtualCursor(this);
   return result;
 }

sql/recover_module/table.h

@@ -103,9 +103,7 @@ class VirtualTable {
   // SQLite handle for this table. The struct is populated and used by SQLite.
   sqlite3_vtab sqlite_table_;
 
-  // SQLite connection for both the recovered table and the virtual table.
-  sqlite3* const sqlite_db_;
-
+  // See the corresponding getters for these members' purpose.
   sqlite3_file* const sqlite_file_;
   const int root_page_id_;
   const int page_size_;
@@ -120,4 +118,4 @@ class VirtualTable {
 }  // namespace recover
 }  // namespace sql
 
-#endif  // SQL_RECOVER_MODULE_TABLE_H_
+#endif  // SQL_RECOVER_MODULE_TABLE_H_