From 95d226d8f52856be192fab2b42d914450af845fc Mon Sep 17 00:00:00 2001
From: Peter Dillinger <peterd@fb.com>
Date: Tue, 24 Dec 2019 18:45:11 -0800
Subject: [PATCH] Fix a clang analyzer report, and 'analyze' make rule (#6244)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Summary:
Clang analyzer was falsely reporting on use of txn=nullptr.
Added a new const variable so that it can properly prune impossible
control flows.

Also, 'make analyze' previously required setting USE_CLANG=1 as an
environment variable, not a make variable, or else compilation errors
like

g++: error: unrecognized command line option ‘-Wshorten-64-to-32’

Now USE_CLANG is not required for 'make analyze' (it's implied) and you
can do an incremental analysis (recompile what has changed) with
'USE_CLANG=1 make analyze_incremental'
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6244

Test Plan: 'make -j24 analyze', 'make crash_test'

Differential Revision: D19225950

Pulled By: pdillinger

fbshipit-source-id: 14f4039aa552228826a2de62b2671450e0fed3cb
---
 Makefile                                |  3 +++
 db_stress_tool/no_batched_ops_stress.cc | 15 +++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index c75c140af..e8fb31c00 100644
--- a/Makefile
+++ b/Makefile
@@ -1097,6 +1097,9 @@ parallel_check: $(TESTS)
 		TMPD=$(TMPD) J=$(J) db_test=0 parloop;
 
 analyze: clean
+	USE_CLANG=1 $(MAKE) analyze_incremental
+
+analyze_incremental:
 	$(CLANG_SCAN_BUILD) --use-analyzer=$(CLANG_ANALYZER) \
 		--use-c++=$(CXX) --use-cc=$(CC) --status-bugs \
 		-o $(CURDIR)/scan_build_report \
diff --git a/db_stress_tool/no_batched_ops_stress.cc b/db_stress_tool/no_batched_ops_stress.cc
index 0523ae0fe..0cc98ed3a 100644
--- a/db_stress_tool/no_batched_ops_stress.cc
+++ b/db_stress_tool/no_batched_ops_stress.cc
@@ -165,21 +165,28 @@ class NonBatchedOpsStressTest : public StressTest {
     std::vector<Status> statuses(num_keys);
     ColumnFamilyHandle* cfh = column_families_[rand_column_families[0]];
 
+    // To appease clang analyzer
+    const bool use_txn = FLAGS_use_txn;
+
     // Create a transaction in order to write some data. The purpose is to
     // exercise WriteBatchWithIndex::MultiGetFromBatchAndDB. The transaction
     // will be rolled back once MultiGet returns.
 #ifndef ROCKSDB_LITE
     Transaction* txn = nullptr;
-    if (FLAGS_use_txn) {
+    if (use_txn) {
       WriteOptions wo;
-      NewTxn(wo, &txn);
+      Status s = NewTxn(wo, &txn);
+      if (!s.ok()) {
+        fprintf(stderr, "NewTxn: %s\n", s.ToString().c_str());
+        std::terminate();
+      }
     }
 #endif
     for (size_t i = 0; i < num_keys; ++i) {
       key_str.emplace_back(Key(rand_keys[i]));
       keys.emplace_back(key_str.back());
 #ifndef ROCKSDB_LITE
-      if (FLAGS_use_txn) {
+      if (use_txn) {
         // With a 1 in 10 probability, insert the just added key in the batch
         // into the transaction. This will create an overlap with the MultiGet
         // keys and exercise some corner cases in the code
@@ -216,7 +223,7 @@ class NonBatchedOpsStressTest : public StressTest {
 #endif
     }
 
-    if (!FLAGS_use_txn) {
+    if (!use_txn) {
       db_->MultiGet(read_opts, cfh, num_keys, keys.data(), values.data(),
                     statuses.data());
     } else {