kernel: allow null data_directory

stickies-v · stickies-v · commit 6657bcbdb4d0 · 2025-11-13T18:17:58.000Z
An empty path may be represented with a nullptr. For example,
std::string_view::data() may return nullptr.

Removes the BITCOINKERNEL_ARG_NONNULL attribute for data_directory,
and instead handles such null arguments in the implementation.

Also documents how BITCOINKERNEL_ARG_NONNULL should be used.
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
@@ -896,6 +896,10 @@ btck_BlockValidationResult btck_block_validation_state_get_block_validation_resu
 
 btck_ChainstateManagerOptions* btck_chainstate_manager_options_create(const btck_Context* context, const char* data_dir, size_t data_dir_len, const char* blocks_dir, size_t blocks_dir_len)
 {
+    if (data_dir == nullptr || data_dir_len == 0 || blocks_dir == nullptr || blocks_dir_len == 0) {
+        LogError("Failed to create chainstate manager options: dir must be non-null and non-empty");
+        return nullptr;
+    }
     try {
         fs::path abs_data_dir{fs::absolute(fs::PathFromString({data_dir, data_dir_len}))};
         fs::create_directories(abs_data_dir);
diff --git a/src/kernel/bitcoinkernel.h b/src/kernel/bitcoinkernel.h
@@ -35,6 +35,17 @@
 #else
     #define BITCOINKERNEL_WARN_UNUSED_RESULT
 #endif
+
+/**
+ * BITCOINKERNEL_ARG_NONNULL is a compiler attribute used to indicate that
+ * certain pointer arguments to a function are not expected to be null.
+ *
+ * Callers must not pass a null pointer for arguments marked with this attribute,
+ * as doing so may result in undefined behavior. This attribute should only be
+ * used for arguments where a null pointer is unambiguously a programmer error,
+ * such as for opaque handles, and not for pointers to raw input data that might
+ * validly be null (e.g., from an empty std::span or std::string).
+ */
 #if !defined(BITCOINKERNEL_BUILD) && defined(__GNUC__)
     #define BITCOINKERNEL_ARG_NONNULL(...) __attribute__((__nonnull__(__VA_ARGS__)))
 #else
@@ -933,19 +944,20 @@ BITCOINKERNEL_API const btck_BlockHash* BITCOINKERNEL_WARN_UNUSED_RESULT btck_bl
  * @brief Create options for the chainstate manager.
  *
  * @param[in] context          Non-null, the created options and through it the chainstate manager will
-                               associate with this kernel context for the duration of their lifetimes.
- * @param[in] data_directory   Non-null, path string of the directory containing the chainstate data.
- *                             If the directory does not exist yet, it will be created.
- * @param[in] blocks_directory Non-null, path string of the directory containing the block data. If
- *                             the directory does not exist yet, it will be created.
+ *                             associate with this kernel context for the duration of their lifetimes.
+ * @param[in] data_directory   Non-null, non-empty path string of the directory containing the
+ *                             chainstate data. If the directory does not exist yet, it will be
+ *                             created.
+ * @param[in] blocks_directory Non-null, non-empty path string of the directory containing the block
+ *                             data. If the directory does not exist yet, it will be created.
  * @return                     The allocated chainstate manager options, or null on error.
  */
 BITCOINKERNEL_API btck_ChainstateManagerOptions* BITCOINKERNEL_WARN_UNUSED_RESULT btck_chainstate_manager_options_create(
     const btck_Context* context,
     const char* data_directory,
     size_t data_directory_len,
     const char* blocks_directory,
-    size_t blocks_directory_len) BITCOINKERNEL_ARG_NONNULL(1, 2);
+    size_t blocks_directory_len) BITCOINKERNEL_ARG_NONNULL(1);
 
 /**
  * @brief Set the number of available worker threads used during validation.
diff --git a/src/kernel/bitcoinkernel_wrapper.h b/src/kernel/bitcoinkernel_wrapper.h
@@ -937,8 +937,9 @@ class Context : public Handle<btck_Context, btck_context_copy, btck_context_dest
 class ChainstateManagerOptions : public UniqueHandle<btck_ChainstateManagerOptions, btck_chainstate_manager_options_destroy>
 {
 public:
-    ChainstateManagerOptions(const Context& context, const std::string& data_dir, const std::string& blocks_dir)
-        : UniqueHandle{btck_chainstate_manager_options_create(context.get(), data_dir.c_str(), data_dir.length(), blocks_dir.c_str(), blocks_dir.length())}
+    ChainstateManagerOptions(const Context& context, std::string_view data_dir, std::string_view blocks_dir)
+        : UniqueHandle{btck_chainstate_manager_options_create(
+              context.get(), data_dir.data(), data_dir.length(), blocks_dir.data(), blocks_dir.length())}
     {
     }
 
diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
@@ -625,6 +625,20 @@ BOOST_AUTO_TEST_CASE(btck_chainman_tests)
         ChainstateManagerOptions chainman_opts{context, test_directory.m_directory.string(), (test_directory.m_directory / "blocks").string()};
         ChainMan chainman{context, chainman_opts};
     }
+    { // null or empty data_directory or blocks_directory are not allowed
+        Context context{};
+        auto valid_dir{test_directory.m_directory.string()};
+        std::vector<std::pair<std::string_view, std::string_view>> illegal_cases{
+            {"", valid_dir},
+            {valid_dir, {nullptr, 0}},
+            {"", ""},
+            {{nullptr, 0}, {nullptr, 0}},
+        };
+        for (auto& [data_dir, blocks_dir] : illegal_cases) {
+            BOOST_CHECK_THROW(ChainstateManagerOptions(context, data_dir, blocks_dir),
+                              std::runtime_error);
+        };
+    }
 
     auto notifications{std::make_shared<TestKernelNotifications>()};
     auto context{create_context(notifications, ChainType::MAINNET)};

Original file line number	Diff line number	Diff line change
`@@ -937,8 +937,9 @@ class Context : public Handle<btck_Context, btck_context_copy, btck_context_dest`
`937`	`937`	`class ChainstateManagerOptions : public UniqueHandle<btck_ChainstateManagerOptions, btck_chainstate_manager_options_destroy>`
`938`	`938`	`{`
`939`	`939`	`public:`
`940`		`- ChainstateManagerOptions(const Context& context, const std::string& data_dir, const std::string& blocks_dir)`
`941`		`- : UniqueHandle{btck_chainstate_manager_options_create(context.get(), data_dir.c_str(), data_dir.length(), blocks_dir.c_str(), blocks_dir.length())}`
	`940`	`+ ChainstateManagerOptions(const Context& context, std::string_view data_dir, std::string_view blocks_dir)`
	`941`	`+ : UniqueHandle{btck_chainstate_manager_options_create(`
	`942`	`+ context.get(), data_dir.data(), data_dir.length(), blocks_dir.data(), blocks_dir.length())}`
`942`	`943`	`{`
`943`	`944`	`}`
`944`	`945`