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

hebasto commented at 8:02 AM on October 5, 2022: member

No description provided.
maflcko approved
maflcko commented at 8:34 AM on October 5, 2022: member

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


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



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.
iwyu: Add zmq source files 13afcc0cd4
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.
hebasto force-pushed on Oct 10, 2022
hebasto commented at 2:47 PM on October 10, 2022: member
Updated 487cc46b5faa994e325d7f1541a49797aa12f9a4 -> 13afcc0cd4c2975852924d2d9be5e96096147716 (pr26254.01 -> pr26254.02, diff):
- addressed @fanquake's [comment](https://github.com/bitcoin/bitcoin/pull/26254#discussion_r990947503
fanquake approved
fanquake commented at 3:16 PM on October 10, 2022: member

ACK 13afcc0cd4c2975852924d2d9be5e96096147716
maflcko merged this on Oct 10, 2022
maflcko closed this on Oct 10, 2022
hebasto deleted the branch on Oct 10, 2022
sidhujag referenced this in commit 8c4816584c on Oct 10, 2022
bitcoin locked this on Oct 10, 2023

Contributors

iwyu: Add zmq source files #26254

Conflicts