kernel: allow null data_directory #33867

1. 
   stickies-v commented at 10:43 pm on November 12, 2025: contributor

   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.

   Follow-up to #33853#pullrequestreview-3454620265

2. kernel: allow null data_directory

   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.

   1dee29ef47
3. DrahtBot added the label Validation on Nov 12, 2025
4. 
   DrahtBot commented at 10:43 pm on November 12, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33867.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   LLM Linter (✨ experimental)

   Possible typos and grammar issues:

   • “will associate” -> “will be associated” [passive voice needed: “the created options and … the chainstate manager will be associated with this kernel context” makes the sentence grammatically correct]

   drahtbot_id_5_m

5. in src/kernel/bitcoinkernel.h:937 in 1dee29ef47

   943@@ -933,10 +944,10 @@ BITCOINKERNEL_API const btck_BlockHash* BITCOINKERNEL_WARN_UNUSED_RESULT btck_bl
   944  * @brief Create options for the chainstate manager.
   945  *
   946  * @param[in] context          Non-null, the created options and through it the chainstate manager will
   947-                               associate with this kernel context for the duration of their lifetimes.
   948- * @param[in] data_directory   Non-null, path string of the directory containing the chainstate data.
   

   ---

   ---

   

   maflcko commented at 7:58 am on November 13, 2025:

   I don’t think this change is right. The path can not be null or empty, because that will fail later on with:

   0unknown location(0): fatal error: in "fs_tests/fsbridge_stem": std::filesystem::__cxx11::filesystem_error: filesystem error: cannot make absolute path: Invalid argument []
   

   ---

   

   stickies-v commented at 11:48 am on November 13, 2025:

   I can’t seem to reproduce that, e.g. these tests all pass for me, indicating that an empty or null path resolves to fs::path(), which then expands to fs::current_path()? How did you get that fatal error?

   0    BOOST_CHECK_EQUAL(fs::PathFromString(""), fs::path());
   1    BOOST_CHECK_EQUAL(fs::PathFromString({nullptr, 0}), fs::path());
   2    BOOST_CHECK_EQUAL(fs::canonical(fs::path()), fs::current_path());
   

   ---

   

   maflcko commented at 12:10 pm on November 13, 2025:

   How did you get that fatal error?

   It is hidden in the error message, but it says “cannot make absolute path”.

   Did you actually call the full kernel function here, including:

   0        fs::path abs_data_dir{fs::absolute(fs::PathFromString({data_dir, data_dir_len}))};
   1        fs::create_directories(abs_data_dir);
   

   In any case, even if it doesn’t crash, I wonder what the valid meaning is here, of passing an empty string?

Contributors
stickies-v DrahtBot maflcko yancyribbens

Labels
Validation