kernel: make btck_LogEntry opaque #36584

pull stickies-v wants to merge 1 commits into bitcoin:master from stickies-v:2026-10/kernel-logentry-opaque changing 3 files +175 −78
  1. stickies-v commented at 2:51 PM on October 20, 2026: contributor

    Makes btck_LogEntry an opaque type with btck_log_entry_get_* accessors, instead of a public struct. This allows adding or changing fields without breaking the ABI, and is consistent with ~all other kernel objects. Follow-up to #34374 (review).

    I added a benchmark in https://github.com/bitcoin/bitcoin/compare/master...stickies-v:bitcoin:2026-10/kernel-logentry-bench, and found that performance is not meaningfully impacted either way. Some results below.

    <details> <summary>Performance stats</summary>

    Performance

    Fields are now converted only when read, at the cost of a library call per field. The benchmark measures a kernel log statement up to and including the callback (Apple M4 Pro, static library, -O2, median of 2×25 interleaved runs):

    Callback reads master PR Delta
    no fields 57.4 ns 55.2 ns −1.6 to −2.5 ns
    message only 57.8 ns 56.0 ns −1.4 to −2.0 ns
    all 9 fields 57.9 ns 62.7 ns +4.8 to +5.4 ns

    Break-even is at 2-3 fields. Either way, the difference is small next to the ~50 ns the log statement costs before the callback, and to what a consumer typically does with the entry (formatting, I/O).

    </details>

  2. kernel: make btck_LogEntry opaque
    Allows adding or changing fields without breaking the ABI, consistent
    with other kernel objects. Strings are returned as btck_StringView.
    
    Fields are now only converted when read, at the cost of a library call
    per field.
    
    The logging callback may now call the btck_log_entry_get_* functions on
    its entry. They only read the entry, so they cannot deadlock.
    b2c077394a
  3. DrahtBot added the label Validation on Oct 20, 2026
  4. DrahtBot commented at 2:52 PM on October 20, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    External sites

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process.

    Type Reviewers
    ACK sedited, musaHaruna

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #34775 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/34775.svg"></sub> (kernel: allow only one global logging connection by stickies-v)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  5. in src/kernel/bitcoinkernel.h:981 in b2c077394a
     976 | +/**
     977 | + * @brief Get the name of the thread that produced the log message.
     978 | + *
     979 | + * @param[in] entry Non-null btck_LogEntry
     980 | + * @return          Thread name
     981 | + */
    


    sedited commented at 10:21 AM on October 21, 2026:

    Might a one-line /// description be better for these trivial getters?


    stickies-v commented at 10:27 AM on October 21, 2026:

    I personally prefer the consistency of using doxygen docstrings for all public functions, but don't care too strongly either way, so happy to change if that's what people prefer. Will leave as-is until more people chime in.

  6. sedited approved
  7. sedited commented at 10:21 AM on October 21, 2026: contributor

    ACK b2c077394a585a4155bd9d7a28b9b7c7553c9dc5

  8. musaHaruna commented at 10:31 AM on October 21, 2026: contributor

    Code Review ACK b2c0773

    Tested locally, including calling all nine getters from the logging callback and checking the message, timestamp, source location, level, and category for the connection and disconnection messages.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-10-27 17:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me