iwyu: Add zmq source files #26254

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:221005-zmq changing 7 files +44 −4
  1. hebasto commented at 8:02 AM on October 5, 2022: member

    No description provided.

  2. maflcko approved
  3. maflcko commented at 8:34 AM on October 5, 2022: member

    lgtm

  4. DrahtBot commented at 5:35 PM on October 5, 2022: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #26251 (refactor: add cs_main.h by fanquake)

    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.

  5. DrahtBot cross-referenced this on Oct 5, 2022 from issue refactor: add kernel/cs_main.h by fanquake
  6. iwyu: Add zmq source files 13afcc0cd4
  7. in src/zmq/zmqpublishnotifier.cpp:32 in 487cc46b5f outdated 
      28 |  #include <cstdarg>
  29 |  #include <cstddef>
  30 | +#include <cstdint>
  31 |  #include <map>
  32 |  #include <optional>
  33 | +#include <string.h>
    fanquake commented at 6:35 AM on October 10, 2022:

    Given you're introducing c* headers everywhere else, you can use cstring here.

    hebasto commented at 1:20 PM on October 10, 2022:

    Everywhere in our codebase we use memcpy and strlen which are declared in the global namespace rather std:: one. Therefore, the C-header string.h is used.

    Replacements s/memcpy/std::memcpy/, s/strlen/std::strlen/ and s/string.h/ctring/ deserve its own PR, aren't they?

    fanquake commented at 2:03 PM on October 10, 2022:

    Everywhere in our codebase we use memcpy and strlen which are declared in the global namespace rather std:: one.

    We use both memcpy and std::memcpy in the codebase. Given we can use memcpy & std::memcpy with cstring, but only memcpy with string.h, and given that we are migrating towards the cpp headers, I don't see why we'd prefer string.h, or newly add it now, just to change it later, when we can just use cstring and acheive the same thing.

    hebasto commented at 2:47 PM on October 10, 2022:

    Thanks. Updated.

  8. hebasto force-pushed on Oct 10, 2022
  9. hebasto commented at 2:47 PM on October 10, 2022: member

    Updated 487cc46b5faa994e325d7f1541a49797aa12f9a4 -> 13afcc0cd4c2975852924d2d9be5e96096147716 (pr26254.01 -> pr26254.02, diff):

  10. fanquake approved
  11. fanquake commented at 3:16 PM on October 10, 2022: member

    ACK 13afcc0cd4c2975852924d2d9be5e96096147716

  12. maflcko merged this on Oct 10, 2022
  13. maflcko closed this on Oct 10, 2022
  14. hebasto deleted the branch on Oct 10, 2022
  15. sidhujag referenced this in commit 8c4816584c on Oct 10, 2022
  16. bitcoin locked this on Oct 10, 2023
Contributors
hebastomaflckoDrahtBotfanquake
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-05-20 06:53 UTC