qqiangwu / cppsafe

Cpp lifetime safety profile static analyzer
MIT License
45 stars 1 forks source link

rocksdb: non-const pointer null check false positive #16

Closed qqiangwu closed 5 months ago

qqiangwu commented 7 months ago
    Transaction* txn2 = nullptr;
    if (has_recent_prepare) {
      txn2 =
          db->BeginTransaction(WriteOptions(), TransactionOptions(), nullptr);
      ASSERT_OK(txn2->SetName("txn2"));
      ASSERT_OK(txn2->Put("key3", "value3"));
      ASSERT_OK(txn2->Prepare());
    }
    // snapshot2 should get min_uncommitted from delayed_prepared_ set.
    auto snapshot2 = db->GetSnapshot();
    ASSERT_EQ(transaction->GetId(),
              ((SnapshotImpl*)snapshot1)->min_uncommitted_);
    ASSERT_OK(transaction->Commit());
    delete transaction;
    if (has_recent_prepare) {
      ASSERT_OK(txn2->Commit());
      delete txn2;
    }

gives:

/Users/wuqq/dev/rocksdb-main/utilities/transactions/write_prepared_transaction_test.cc:2567:17: warning: passing a null pointer as argument where a non-null pointer is expected
 2567 |       ASSERT_OK(txn2->Commit());
      |                 ^~~~
/Users/wuqq/dev/rocksdb-main/test_util/testharness.h:83:62: note: expanded from macro 'ASSERT_OK'
   83 |   ASSERT_PRED_FORMAT1(ROCKSDB_NAMESPACE::test::AssertStatus, s)
      |                                                              ^
/Users/wuqq/dev/rocksdb-main/third-party/gtest-1.8.1/fused-src/gtest/gtest.h:19909:36: note: expanded from macro 'ASSERT_PRED_FORMAT1'
 19909 |   GTEST_PRED_FORMAT1_(pred_format, v1, GTEST_FATAL_FAILURE_)
       |                                    ^~
/Users/wuqq/dev/rocksdb-main/third-party/gtest-1.8.1/fused-src/gtest/gtest.h:19892:34: note: expanded from macro 'GTEST_PRED_FORMAT1_'
 19892 |   GTEST_ASSERT_(pred_format(#v1, v1), \
       |                                  ^~
/Users/wuqq/dev/rocksdb-main/third-party/gtest-1.8.1/fused-src/gtest/gtest.h:19868:52: note: expanded from macro 'GTEST_ASSERT_'
 19868 |   if (const ::testing::AssertionResult gtest_ar = (expression)) \
       |                                                    ^~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/utilities/transactions/write_prepared_transaction_test.cc:2552:5: note: assigned here
 2552 |     Transaction* txn2 = nullptr;
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~

add an assert can fix the warning, but do we have a better choice?

qqiangwu commented 5 months ago

disable it by default, in the future we will use static analyzer