Cleanup headers after #20788 #22952

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:210911-headers changing 2 files +5 −5

hebasto commented at 9:14 AM on September 11, 2021: member

This is a header cleanup after #20788.
Cleanup headers after #20788 3174425255
hebasto commented at 9:15 AM on September 11, 2021: member

cc @vasild
DrahtBot added the label P2P on Sep 11, 2021
DrahtBot added the label Utils/log/libs on Sep 11, 2021
MarcoFalke removed the label P2P on Sep 11, 2021
MarcoFalke removed the label Utils/log/libs on Sep 11, 2021
MarcoFalke added the label Refactoring on Sep 11, 2021
vasild commented at 10:47 AM on September 13, 2021: member

Just out of curiosity - what drove you to do this change? Did you use some tool which suggested these changes?

It is somewhat difficult to asses whether removing #include <asysheader> from foo.cpp is justified - how to check that foo.cpp is not using any of the functions from that header?

I see why <codecvt> was moved from netbase.cpp to util/sock.cpp - the windows-only variant of NetworkErrorString() is using std::codecvt_utf8_utf16 and that function was moved from netbase.cpp to util/sock.cpp (but the header was not moved).

Why remove <cwchar> from util/sock.cpp? Looks like in order to confirm this change is legit one has to verify that none of the symbols from https://en.cppreference.com/w/cpp/header/cwchar are used in util/sock.cpp. Is there an easy way to do that?

Same for making <locale> windows-only. How to check that non-windows code is not using any of its symbols?
hebasto commented at 11:09 AM on September 13, 2021: member

@vasild

Just out of curiosity - what drove you to do this change?

Reading dec9b5e850c6aad989e814aea5b630b36f55d580 commit while working on #20744.

Did you use some tool which suggested these changes?

No. The added code is short enough to manually verify need of #include <cwchar> and #include <locale>.

Why remove <cwchar> from util/sock.cpp?

Why was it added in dec9b5e850c6aad989e814aea5b630b36f55d580? I see no reason for that.

Same for making <locale> windows-only. How to check that non-windows code is not using any of its symbols?

Same. I see no reason to #include <locale> for non-Windows platforms in dec9b5e850c6aad989e814aea5b630b36f55d580.
vasild approved
vasild commented at 11:03 AM on September 14, 2021: member

ACK 317442525586ba9ff8b9af6c506b48f87cd8cd87

Alright, thanks for the clarifications, @hebasto. I don't remember why I added <cwchar>, surely it was not without a reason, maybe some windows CI failed on me. Anyway, now it looks like it is not needed and CI is green.

Is it time to integrate https://include-what-you-use.org/ into the project?
MarcoFalke added the label DrahtBot Guix build requested on Sep 14, 2021
hebasto commented at 12:11 PM on September 14, 2021: member

Is it time to integrate https://include-what-you-use.org/ into the project?

#15442

DrahtBot commented at 5:12 PM on September 15, 2021: member

Guix builds

File	commit 2161a058552ac938f2079b311a2d12f5d1772d01<br>(master)	commit 629b86f7132e8634c20684bea80e073a606e37b1<br>(master and this pull)
SHA256SUMS.part	`a9a375ae5d3d7897...`	`63a66ef0d48aa319...`
*-aarch64-linux-gnu-debug.tar.gz	`0ffaab33cf740e46...`	`a0d36982affe1785...`
*-aarch64-linux-gnu.tar.gz	`b41ca36d3fed9c2f...`	`b28537bb715e434c...`
*-arm-linux-gnueabihf-debug.tar.gz	`1ac79064c91854ac...`	`b282ec23a240851e...`
*-arm-linux-gnueabihf.tar.gz	`5399e4a5ea61ccd1...`	`a2abefc08cf75454...`
*-osx-unsigned.dmg	`1ca70ce0b4b48e27...`	`0d782c6ef8ad5027...`
*-osx-unsigned.tar.gz	`e7d44f030cbdbd03...`	`61b51e3f1ba798e1...`
*-osx64.tar.gz	`de52a250ed0768cd...`	`ee042cc7919f0bdc...`
*-powerpc64-linux-gnu-debug.tar.gz	`0966df174570e7b6...`	`f01146ec5f4f6b31...`
*-powerpc64-linux-gnu.tar.gz	`465af3689d999d1f...`	`682c6f17c21d1926...`
*-powerpc64le-linux-gnu-debug.tar.gz	`639887f5ab3a24eb...`	`ed84b47d2320091e...`
*-powerpc64le-linux-gnu.tar.gz	`ba92b866267f368e...`	`95b7528cb2539f77...`
*-riscv64-linux-gnu-debug.tar.gz	`5e51f8e7ca52cf5c...`	`7bb6710a7fe75aba...`
*-riscv64-linux-gnu.tar.gz	`3d362954996f46b2...`	`9285f0a685396098...`
*-win-unsigned.tar.gz	`2fe1344f94a4ec4c...`	`fde7b27d2a848815...`
*-win64-debug.zip	`e0054a82e946aa6c...`	`55e77671a14ccebc...`
*-win64-setup-unsigned.exe	`e327f17bc9c9af84...`	`beeadf2acf423425...`
*-win64.zip	`f649f697b0dbad16...`	`89ee5423b1664e47...`
*-x86_64-linux-gnu-debug.tar.gz	`daf5d05b8b85d09a...`	`150c6c36e3100936...`
*-x86_64-linux-gnu.tar.gz	`9930b7ae5591f1d8...`	`af015fbb7894a20b...`
*.tar.gz	`8815d09678e076d6...`	`05935b765c18b5f6...`
guix_build.log	`646ed421098563a1...`	`3be591ba7433c27f...`
guix_build.log.diff		`6e22cfdf51222c3b...`

DrahtBot removed the label DrahtBot Guix build requested on Sep 15, 2021
MarcoFalke referenced this in commit ec7ec69c7b on Sep 16, 2021
DrahtBot commented at 7:40 AM on September 16, 2021: member



🐙 This pull request conflicts with the target branch and needs rebase.

<sub>Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".</sub>
DrahtBot added the label Needs rebase on Sep 16, 2021
fanquake closed this on Sep 16, 2021
hebasto deleted the branch on Sep 16, 2021
MarcoFalke commented at 9:07 AM on September 16, 2021: member

This was merged, but not marked as merged by GitHub.
sidhujag referenced this in commit c6525c9b58 on Sep 19, 2021
DrahtBot locked this on Oct 30, 2022

Contributors

Labels

Refactoring Needs rebase

Linked (view graph)

#20788 net: add RAII socket and use it instead of bare SOCKET