Merge #20432: net: Treat raw message bytes as uint8_t

fabecce71909c984504c21fa05f91d5f1b471e8c net: Treat raw message bytes as uint8_t (MarcoFalke)

Pull request description:

  Using `uint8_t` from the beginning when messages are `recv`ed has two style benefits:
  * The signedness is clear from reading the code, as it does not depend on the architecture
  * When passing the bytes on, the need for static signedness casts is dropped, making the code a bit less verbose and more coherent

ACKs for top commit:
  laanwj:
    Code review ACK fabecce71909c984504c21fa05f91d5f1b471e8c
  theStack:
    Code Review ACK fabecce71909c984504c21fa05f91d5f1b471e8c
  jonatack:
    Tested ACK fabecce71909c984504c21fa05f91d5f1b471e8c

Tree-SHA512: e6d9803c78633fde3304faf592afa961ff9462a7912d1da97a24720265274aa10ab4168d71b6ec2756b7448dd42585321afee0e5c889e705be778ce9a330d145
This commit is contained in:
Wladimir J. van der Laan 2020-11-23 10:25:56 +01:00 committed by PastaPastaPasta
parent 21a6b171dc
commit c1b5ec6360
6 changed files with 19 additions and 19 deletions

View File

@ -676,7 +676,7 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
} }
#undef X #undef X
bool CNode::ReceiveMsgBytes(Span<const char> msg_bytes, bool& complete) bool CNode::ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete)
{ {
complete = false; complete = false;
int64_t nTimeMicros = GetTimeMicros(); int64_t nTimeMicros = GetTimeMicros();
@ -747,7 +747,7 @@ int CNode::GetSendVersion() const
return nSendVersion; return nSendVersion;
} }
int V1TransportDeserializer::readHeader(Span<const char> msg_bytes) int V1TransportDeserializer::readHeader(Span<const uint8_t> msg_bytes)
{ {
// copy data to temporary parsing buffer // copy data to temporary parsing buffer
unsigned int nRemaining = 24 - nHdrPos; unsigned int nRemaining = 24 - nHdrPos;
@ -787,7 +787,7 @@ int V1TransportDeserializer::readHeader(Span<const char> msg_bytes)
return nCopy; return nCopy;
} }
int V1TransportDeserializer::readData(Span<const char> msg_bytes) int V1TransportDeserializer::readData(Span<const uint8_t> msg_bytes)
{ {
unsigned int nRemaining = hdr.nMessageSize - nDataPos; unsigned int nRemaining = hdr.nMessageSize - nDataPos;
unsigned int nCopy = std::min<unsigned int>(nRemaining, msg_bytes.size()); unsigned int nCopy = std::min<unsigned int>(nRemaining, msg_bytes.size());
@ -797,7 +797,7 @@ int V1TransportDeserializer::readData(Span<const char> msg_bytes)
vRecv.resize(std::min(hdr.nMessageSize, nDataPos + nCopy + 256 * 1024)); vRecv.resize(std::min(hdr.nMessageSize, nDataPos + nCopy + 256 * 1024));
} }
hasher.Write(MakeUCharSpan(msg_bytes.first(nCopy))); hasher.Write(msg_bytes.first(nCopy));
memcpy(&vRecv[nDataPos], msg_bytes.data(), nCopy); memcpy(&vRecv[nDataPos], msg_bytes.data(), nCopy);
nDataPos += nCopy; nDataPos += nCopy;
@ -1912,13 +1912,13 @@ void CConnman::SocketHandler()
size_t CConnman::SocketRecvData(CNode *pnode) size_t CConnman::SocketRecvData(CNode *pnode)
{ {
// typical socket buffer is 8K-64K // typical socket buffer is 8K-64K
char pchBuf[0x10000]; uint8_t pchBuf[0x10000];
int nBytes = 0; int nBytes = 0;
{ {
LOCK(pnode->cs_hSocket); LOCK(pnode->cs_hSocket);
if (pnode->hSocket == INVALID_SOCKET) if (pnode->hSocket == INVALID_SOCKET)
return 0; return 0;
nBytes = recv(pnode->hSocket, pchBuf, sizeof(pchBuf), MSG_DONTWAIT); nBytes = recv(pnode->hSocket, (char*)pchBuf, sizeof(pchBuf), MSG_DONTWAIT);
if (nBytes < (int)sizeof(pchBuf)) { if (nBytes < (int)sizeof(pchBuf)) {
pnode->fHasRecvData = false; pnode->fHasRecvData = false;
} }
@ -1926,7 +1926,7 @@ size_t CConnman::SocketRecvData(CNode *pnode)
if (nBytes > 0) if (nBytes > 0)
{ {
bool notify = false; bool notify = false;
if (!pnode->ReceiveMsgBytes(Span<const char>(pchBuf, nBytes), notify)) { if (!pnode->ReceiveMsgBytes(Span<const uint8_t>(pchBuf, nBytes), notify)) {
LOCK(cs_vNodes); LOCK(cs_vNodes);
pnode->CloseSocketDisconnect(this); pnode->CloseSocketDisconnect(this);
} }

View File

@ -934,7 +934,7 @@ public:
// set the serialization context version // set the serialization context version
virtual void SetVersion(int version) = 0; virtual void SetVersion(int version) = 0;
/** read and deserialize data, advances msg_bytes data pointer */ /** read and deserialize data, advances msg_bytes data pointer */
virtual int Read(Span<const char>& msg_bytes) = 0; virtual int Read(Span<const uint8_t>& msg_bytes) = 0;
// decomposes a message from the context // decomposes a message from the context
virtual std::optional<CNetMessage> GetMessage(int64_t time, uint32_t& out_err) = 0; virtual std::optional<CNetMessage> GetMessage(int64_t time, uint32_t& out_err) = 0;
virtual ~TransportDeserializer() {} virtual ~TransportDeserializer() {}
@ -955,8 +955,8 @@ private:
unsigned int nDataPos; unsigned int nDataPos;
const uint256& GetMessageHash() const; const uint256& GetMessageHash() const;
int readHeader(Span<const char> msg_bytes); int readHeader(Span<const uint8_t> msg_bytes);
int readData(Span<const char> msg_bytes); int readData(Span<const uint8_t> msg_bytes);
void Reset() { void Reset() {
vRecv.clear(); vRecv.clear();
@ -990,7 +990,7 @@ public:
hdrbuf.SetVersion(nVersionIn); hdrbuf.SetVersion(nVersionIn);
vRecv.SetVersion(nVersionIn); vRecv.SetVersion(nVersionIn);
} }
int Read(Span<const char>& msg_bytes) override int Read(Span<const uint8_t>& msg_bytes) override
{ {
int ret = in_data ? readData(msg_bytes) : readHeader(msg_bytes); int ret = in_data ? readData(msg_bytes) : readHeader(msg_bytes);
if (ret < 0) { if (ret < 0) {
@ -1285,7 +1285,7 @@ public:
* @return True if the peer should stay connected, * @return True if the peer should stay connected,
* False if the peer should be disconnected from. * False if the peer should be disconnected from.
*/ */
bool ReceiveMsgBytes(Span<const char> msg_bytes, bool& complete); bool ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete);
void SetRecvVersion(int nVersionIn) void SetRecvVersion(int nVersionIn)
{ {

View File

@ -129,7 +129,7 @@ FUZZ_TARGET_INIT(net, initialize_net)
[&] { [&] {
const std::vector<uint8_t> b = ConsumeRandomLengthByteVector(fuzzed_data_provider); const std::vector<uint8_t> b = ConsumeRandomLengthByteVector(fuzzed_data_provider);
bool complete; bool complete;
node.ReceiveMsgBytes({(const char*)b.data(), b.size()}, complete); node.ReceiveMsgBytes(b, complete);
}); });
} }

View File

@ -22,7 +22,7 @@ FUZZ_TARGET_INIT(p2p_transport_deserializer, initialize_p2p_transport_deserializ
{ {
// Construct deserializer, with a dummy NodeId // Construct deserializer, with a dummy NodeId
V1TransportDeserializer deserializer{Params(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION}; V1TransportDeserializer deserializer{Params(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION};
Span<const char> msg_bytes{(const char*)buffer.data(), buffer.size()}; Span<const uint8_t> msg_bytes{buffer};
while (msg_bytes.size() > 0) { while (msg_bytes.size() > 0) {
const int handled = deserializer.Read(msg_bytes); const int handled = deserializer.Read(msg_bytes);
if (handled < 0) { if (handled < 0) {

View File

@ -7,7 +7,7 @@
#include <chainparams.h> #include <chainparams.h>
#include <net.h> #include <net.h>
void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, Span<const char> msg_bytes, bool& complete) const void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_bytes, bool& complete) const
{ {
assert(node.ReceiveMsgBytes(msg_bytes, complete)); assert(node.ReceiveMsgBytes(msg_bytes, complete));
if (complete) { if (complete) {
@ -29,11 +29,11 @@ void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, Span<const char> msg_bytes
bool ConnmanTestMsg::ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) const bool ConnmanTestMsg::ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) const
{ {
std::vector<unsigned char> ser_msg_header; std::vector<uint8_t> ser_msg_header;
node.m_serializer->prepareForTransport(ser_msg, ser_msg_header); node.m_serializer->prepareForTransport(ser_msg, ser_msg_header);
bool complete; bool complete;
NodeReceiveMsgBytes(node, {(const char*)ser_msg_header.data(), ser_msg_header.size()}, complete); NodeReceiveMsgBytes(node, ser_msg_header, complete);
NodeReceiveMsgBytes(node, {(const char*)ser_msg.data.data(), ser_msg.data.size()}, complete); NodeReceiveMsgBytes(node, ser_msg.data, complete);
return complete; return complete;
} }

View File

@ -31,7 +31,7 @@ struct ConnmanTestMsg : public CConnman {
void ProcessMessagesOnce(CNode& node) { m_msgproc->ProcessMessages(&node, flagInterruptMsgProc); } void ProcessMessagesOnce(CNode& node) { m_msgproc->ProcessMessages(&node, flagInterruptMsgProc); }
void NodeReceiveMsgBytes(CNode& node, Span<const char> msg_bytes, bool& complete) const; void NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_bytes, bool& complete) const;
bool ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) const; bool ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) const;
}; };