net, refactor: Rename CNetMessage::m_command with CNetMessage::m_type #24078

pull hebasto wants to merge 2 commits into bitcoin:master from hebasto:220115-message changing 4 files +12 −16

hebasto commented at 7:09 PM on January 15, 2022: member

#18533#issue-594592488:

a message is not a command, but simply a message of some type

Continuation of bitcoin/bitcoin#18533 and bitcoin/bitcoin#18937.

scripted-diff: Rename CNetMessage::m_command with CNetMessage::m_type

-BEGIN VERIFY SCRIPT-
sed -i 's/std::string m_command;/std::string m_type;/g' ./src/net.h
sed -i 's/* command and size./* type and size./g' ./src/net.h
sed -i 's/msg.m_command/msg.m_type/g' ./src/net.cpp ./src/net_processing.cpp ./src/test/fuzz/p2p_transport_serialization.cpp
-END VERIFY SCRIPT-

3073a9917b

net, refactor: Drop tautological local variables 224d87855e
DrahtBot added the label P2P on Jan 15, 2022
theStack approved
theStack commented at 1:52 AM on January 16, 2022: contributor

Concept and code-review ACK 224d87855ec38cc15866d9673e1b19942a82c1cd

There are still quite a few instances of the confusing "command" terminology for message types around, as $ git grep -i command ./src/net* shows. Happy to review further changes in this direction (either in this PR or a follow-up).
DrahtBot commented at 2:44 AM on January 16, 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:
- #23233 (BIP324: Add encrypted p2p transport {de}serializer by dhruv)
- #22053 (refactor: Release cs_main before MaybeSendFeefilter by MarcoFalke)
- #21527 (net_processing: lock clean up by ajtowns)
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.
DrahtBot cross-referenced this on Jan 16, 2022 from issue BIP324: Add encrypted p2p transport {de}serializer by dhruv
DrahtBot cross-referenced this on Jan 16, 2022 from issue refactor: Release cs_main before MaybeSendFeefilter by MarcoFalke
DrahtBot cross-referenced this on Jan 16, 2022 from issue net_processing: lock clean up by ajtowns
shaavan approved
shaavan commented at 2:18 PM on January 20, 2022: contributor
Code Review ACK 224d87855ec38cc15866d9673e1b19942a82c1cd

Going commit wise:
1. I agree with this statement, and hence with the change:
a message is not a command, but simply a message of some type
1. Removing these redundant variables makes the code more clean and readable.
w0xlt approved
w0xlt commented at 3:04 PM on January 23, 2022: contributor

crACK 224d878
MarcoFalke merged this on Jan 24, 2022
MarcoFalke closed this on Jan 24, 2022
hebasto deleted the branch on Jan 24, 2022

in src/net.cpp:749 in 224d87855e

 746 | @@ -747,7 +747,7 @@ CNetMessage V1TransportDeserializer::GetMessage(const std::chrono::microseconds
 747 |      CNetMessage msg(std::move(vRecv));
 748 |  
 749 |      // store command string, time, and sizes

MarcoFalke commented at 12:57 PM on January 24, 2022:

I think there are a few comments which could be updated.

git grep --ignore-case command ./src/net*

shaavan commented at 1:43 PM on January 24, 2022:

Let me take it up as a follow-up.

shaavan cross-referenced this on Jan 24, 2022 from issue Rename message_command variables in src/net* and src/rpc/net.cpp by shaavan
sidhujag referenced this in commit b135edda87 on Jan 28, 2022
MarcoFalke referenced this in commit 0d080a183b on May 5, 2022
sidhujag referenced this in commit aebd7a230d on May 5, 2022
bitcoin locked this on Jan 24, 2023

Contributors

Labels

Linked (view graph)

#18533 scripted-diff: Replace strCommand with msg_type #18937 refactor: s/command/msg_type/ in CNetMsgMaker and CSerializedNetMsg #24141 Rename message_command variables in src/net* and src/rpc/net.cpp #31163 scripted-diff: get rid of remaining "command" terminology in protocol.{h,cpp}#34203 doc: p2p: replace last remaining "command" terminology with "message type"

net, refactor: Rename CNetMessage::m_command with CNetMessage::m_type #24078

Conflicts