Fix CLANG Analyze

Summary:
clang analyze shows warnings after we upgrade the CLANG version. Fix them.
Closes https://github.com/facebook/rocksdb/pull/2839

Differential Revision: D5769060

Pulled By: siying

fbshipit-source-id: 3f8e4df715590d8984f6564b608fa08cfdfa5f14

db/compaction_job.cc

@@ -1067,16 +1067,18 @@ Status CompactionJob::FinishCompactionOutputFile(
     range_del_agg->AddToBuilder(sub_compact->builder.get(), lower_bound,
                                 upper_bound, meta, range_del_out_stats,
                                 bottommost_level_);
+    meta->marked_for_compaction = sub_compact->builder->NeedCompact();
   }
   const uint64_t current_entries = sub_compact->builder->NumEntries();
-  meta->marked_for_compaction = sub_compact->builder->NeedCompact();
   if (s.ok()) {
     s = sub_compact->builder->Finish();
   } else {
     sub_compact->builder->Abandon();
   }
   const uint64_t current_bytes = sub_compact->builder->FileSize();
-  meta->fd.file_size = current_bytes;
+  if (s.ok()) {
+    meta->fd.file_size = current_bytes;
+  }
   sub_compact->current_output()->finished = true;
   sub_compact->total_bytes += current_bytes;
 
@@ -1144,17 +1146,24 @@ Status CompactionJob::FinishCompactionOutputFile(
                      meta->marked_for_compaction ? " (need compaction)" : "");
     }
   }
-  std::string fname = TableFileName(db_options_.db_paths, meta->fd.GetNumber(),
+  std::string fname;
-                                    meta->fd.GetPathId());
+  FileDescriptor output_fd;
+  if (meta != nullptr) {
+    fname = TableFileName(db_options_.db_paths, meta->fd.GetNumber(),
+                          meta->fd.GetPathId());
+    output_fd = meta->fd;
+  } else {
+    fname = "(nil)";
+  }
   EventHelpers::LogAndNotifyTableFileCreationFinished(
       event_logger_, cfd->ioptions()->listeners, dbname_, cfd->GetName(), fname,
-      job_id_, meta->fd, tp, TableFileCreationReason::kCompaction, s);
+      job_id_, output_fd, tp, TableFileCreationReason::kCompaction, s);
 
 #ifndef ROCKSDB_LITE
   // Report new file to SstFileManagerImpl
   auto sfm =
       static_cast<SstFileManagerImpl*>(db_options_.sst_file_manager.get());
-  if (sfm && meta->fd.GetPathId() == 0) {
+  if (sfm && meta != nullptr && meta->fd.GetPathId() == 0) {
     auto fn = TableFileName(cfd->ioptions()->db_paths, meta->fd.GetNumber(),
                             meta->fd.GetPathId());
     sfm->OnAddFile(fn);

db/db_impl.cc

@@ -777,9 +777,7 @@ void DBImpl::BackgroundCallPurge() {
       purge_queue_.pop_front();
 
       mutex_.Unlock();
-      Status file_deletion_status;
+      DeleteObsoleteFileImpl(job_id, fname, type, number, path_id);
-      DeleteObsoleteFileImpl(file_deletion_status, job_id, fname, type, number,
-                             path_id);
       mutex_.Lock();
     } else {
       assert(!logs_to_free_queue_.empty());

db/db_impl.h

@@ -685,9 +685,8 @@ class DBImpl : public DB {
   // Delete any unneeded files and stale in-memory entries.
   void DeleteObsoleteFiles();
   // Delete obsolete files and log status and information of file deletion
-  void DeleteObsoleteFileImpl(Status file_deletion_status, int job_id,
+  void DeleteObsoleteFileImpl(int job_id, const std::string& fname,
-                              const std::string& fname, FileType type,
+                              FileType type, uint64_t number, uint32_t path_id);
-                              uint64_t number, uint32_t path_id);
 
   // Background process needs to call
   //     auto x = CaptureCurrentFileNumberInPendingOutputs()

db/db_impl_files.cc

@@ -300,9 +300,10 @@ bool CompareCandidateFile(const JobContext::CandidateFileInfo& first,
 };  // namespace
 
 // Delete obsolete files and log status and information of file deletion
-void DBImpl::DeleteObsoleteFileImpl(Status file_deletion_status, int job_id,
+void DBImpl::DeleteObsoleteFileImpl(int job_id, const std::string& fname,
-                                    const std::string& fname, FileType type,
+                                    FileType type, uint64_t number,
-                                    uint64_t number, uint32_t path_id) {
+                                    uint32_t path_id) {
+  Status file_deletion_status;
   if (type == kTableFile) {
     file_deletion_status =
         DeleteSSTFile(&immutable_db_options_, fname, path_id);
@@ -488,8 +489,7 @@ void DBImpl::PurgeObsoleteFiles(const JobContext& state, bool schedule_only) {
       InstrumentedMutexLock guard_lock(&mutex_);
       SchedulePendingPurge(fname, type, number, path_id, state.job_id);
     } else {
-      DeleteObsoleteFileImpl(file_deletion_status, state.job_id, fname, type,
+      DeleteObsoleteFileImpl(state.job_id, fname, type, number, path_id);
-                             number, path_id);
     }
   }
 

memtable/inlineskiplist.h

@@ -483,11 +483,13 @@ InlineSkipList<Comparator>::FindLessThan(const char* key, Node** prev,
   // KeyIsAfter(key, last_not_after) is definitely false
   Node* last_not_after = nullptr;
   while (true) {
+    assert(x != nullptr);
     Node* next = x->Next(level);
     assert(x == head_ || next == nullptr || KeyIsAfterNode(next->Key(), x));
     assert(x == head_ || KeyIsAfterNode(key, x));
     if (next != last_not_after && KeyIsAfterNode(key, next)) {
       // Keep searching in this list
+      assert(next != nullptr);
       x = next;
     } else {
       if (prev != nullptr) {
@@ -863,6 +865,7 @@ void InlineSkipList<Comparator>::TEST_Validate() const {
   // levels.
   Node* nodes[kMaxPossibleHeight];
   int max_height = GetMaxHeight();
+  assert(max_height > 0);
   for (int i = 0; i < max_height; i++) {
     nodes[i] = head_;
   }
@@ -892,7 +895,7 @@ void InlineSkipList<Comparator>::TEST_Validate() const {
     }
   }
   for (int i = 1; i < max_height; i++) {
-    assert(nodes[i]->Next(i) == nullptr);
+    assert(nodes[i] != nullptr && nodes[i]->Next(i) == nullptr);
   }
 }
 

memtable/skiplist.h

@@ -310,6 +310,7 @@ typename SkipList<Key, Comparator>::Node* SkipList<Key, Comparator>::
   int level = GetMaxHeight() - 1;
   Node* last_bigger = nullptr;
   while (true) {
+    assert(x != nullptr);
     Node* next = x->Next(level);
     // Make sure the lists are sorted
     assert(x == head_ || next == nullptr || KeyIsAfterNode(next->key, x));
@@ -338,6 +339,7 @@ SkipList<Key, Comparator>::FindLessThan(const Key& key, Node** prev) const {
   // KeyIsAfter(key, last_not_after) is definitely false
   Node* last_not_after = nullptr;
   while (true) {
+    assert(x != nullptr);
     Node* next = x->Next(level);
     assert(x == head_ || next == nullptr || KeyIsAfterNode(next->key, x));
     assert(x == head_ || KeyIsAfterNode(key, x));

utilities/backupable/backupable_db_test.cc

@@ -843,7 +843,7 @@ TEST_F(BackupableDBTest, NoDoubleCopy) {
   ASSERT_OK(test_backup_env_->FileExists(backupdir_ + "/shared/00015.sst"));
 
   // MANIFEST file size should be only 100
-  uint64_t size;
+  uint64_t size = 0;
   test_backup_env_->GetFileSize(backupdir_ + "/private/2/MANIFEST-01", &size);
   ASSERT_EQ(100UL, size);
   test_backup_env_->GetFileSize(backupdir_ + "/shared/00015.sst", &size);