Network: Remove static buffer form of NetworkAddress::GetAddressAsString

This is used in multiple threads concurrently
This commit is contained in:
Jonathan G Rennison
2020-05-06 23:23:03 +01:00
parent af09391bfb
commit ef7e658dee
7 changed files with 45 additions and 31 deletions

View File

@@ -98,12 +98,10 @@ void NetworkAddress::GetAddressAsString(char *buffer, const char *last, bool wit
* @return the address * @return the address
* @note NOT thread safe * @note NOT thread safe
*/ */
const char *NetworkAddress::GetAddressAsString(bool with_family) const char *NetworkAddressDumper::GetAddressAsString(NetworkAddress *addr, bool with_family)
{ {
/* 6 = for the : and 5 for the decimal port number */ addr->GetAddressAsString(this->buf, lastof(this->buf), with_family);
static char buf[NETWORK_HOSTNAME_LENGTH + 6 + 7]; return this->buf;
this->GetAddressAsString(buf, lastof(buf), with_family);
return buf;
} }
/** /**
@@ -289,7 +287,8 @@ static SOCKET ConnectLoopProc(addrinfo *runp)
{ {
const char *type = NetworkAddress::SocketTypeAsString(runp->ai_socktype); const char *type = NetworkAddress::SocketTypeAsString(runp->ai_socktype);
const char *family = NetworkAddress::AddressFamilyAsString(runp->ai_family); const char *family = NetworkAddress::AddressFamilyAsString(runp->ai_family);
const char *address = NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString(); char address[NETWORK_HOSTNAME_LENGTH + 6 + 7];
NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString(address, lastof(address));
SOCKET sock = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol); SOCKET sock = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol);
if (sock == INVALID_SOCKET) { if (sock == INVALID_SOCKET) {
@@ -319,7 +318,7 @@ static SOCKET ConnectLoopProc(addrinfo *runp)
*/ */
SOCKET NetworkAddress::Connect() SOCKET NetworkAddress::Connect()
{ {
DEBUG(net, 1, "Connecting to %s", this->GetAddressAsString()); DEBUG(net, 1, "Connecting to %s", NetworkAddressDumper().GetAddressAsString(this));
return this->Resolve(AF_UNSPEC, SOCK_STREAM, AI_ADDRCONFIG, nullptr, ConnectLoopProc); return this->Resolve(AF_UNSPEC, SOCK_STREAM, AI_ADDRCONFIG, nullptr, ConnectLoopProc);
} }
@@ -333,7 +332,8 @@ static SOCKET ListenLoopProc(addrinfo *runp)
{ {
const char *type = NetworkAddress::SocketTypeAsString(runp->ai_socktype); const char *type = NetworkAddress::SocketTypeAsString(runp->ai_socktype);
const char *family = NetworkAddress::AddressFamilyAsString(runp->ai_family); const char *family = NetworkAddress::AddressFamilyAsString(runp->ai_family);
const char *address = NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString(); char address[NETWORK_HOSTNAME_LENGTH + 6 + 7];
NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString(address, lastof(address));
SOCKET sock = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol); SOCKET sock = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol);
if (sock == INVALID_SOCKET) { if (sock == INVALID_SOCKET) {

View File

@@ -91,7 +91,6 @@ public:
const char *GetHostname(); const char *GetHostname();
void GetAddressAsString(char *buffer, const char *last, bool with_family = true); void GetAddressAsString(char *buffer, const char *last, bool with_family = true);
const char *GetAddressAsString(bool with_family = true);
const sockaddr_storage *GetAddress(); const sockaddr_storage *GetAddress();
/** /**
@@ -179,4 +178,17 @@ public:
static const char *AddressFamilyAsString(int family); static const char *AddressFamilyAsString(int family);
}; };
/**
* The use of a struct is so that when used as an argument to /seprintf/etc, the buffer lives
* on the stack with a lifetime which lasts until the end of the statement.
* This avoids using a static buffer which is thread-unsafe, or needing to call malloc, which would then nee to be freed.
*/
struct NetworkAddressDumper {
const char *GetAddressAsString(NetworkAddress *addr, bool with_family = true);
private:
/* 6 = for the : and 5 for the decimal port number */
char buf[NETWORK_HOSTNAME_LENGTH + 6 + 7];
};
#endif /* NETWORK_CORE_ADDRESS_H */ #endif /* NETWORK_CORE_ADDRESS_H */

View File

@@ -171,9 +171,9 @@ bool NetworkContentSocketHandler::HandlePacket(Packet *p)
default: default:
if (this->HasClientQuit()) { if (this->HasClientQuit()) {
DEBUG(net, 0, "[tcp/content] received invalid packet type %d from %s", type, this->client_addr.GetAddressAsString()); DEBUG(net, 0, "[tcp/content] received invalid packet type %d from %s", type, NetworkAddressDumper().GetAddressAsString(&(this->client_addr)));
} else { } else {
DEBUG(net, 0, "[tcp/content] received illegal packet from %s", this->client_addr.GetAddressAsString()); DEBUG(net, 0, "[tcp/content] received illegal packet from %s", NetworkAddressDumper().GetAddressAsString(&(this->client_addr)));
} }
return false; return false;
} }
@@ -223,7 +223,7 @@ bool NetworkContentSocketHandler::ReceivePackets()
*/ */
bool NetworkContentSocketHandler::ReceiveInvalidPacket(PacketContentType type) bool NetworkContentSocketHandler::ReceiveInvalidPacket(PacketContentType type)
{ {
DEBUG(net, 0, "[tcp/content] received illegal packet type %d from %s", type, this->client_addr.GetAddressAsString()); DEBUG(net, 0, "[tcp/content] received illegal packet type %d from %s", type, NetworkAddressDumper().GetAddressAsString(&(this->client_addr)));
return false; return false;
} }

View File

@@ -135,10 +135,10 @@ void NetworkUDPSocketHandler::SendPacket(Packet *p, NetworkAddress *recv, bool a
/* Send the buffer */ /* Send the buffer */
int res = sendto(s.second, (const char*)p->buffer, p->size, 0, (const struct sockaddr *)send.GetAddress(), send.GetAddressLength()); int res = sendto(s.second, (const char*)p->buffer, p->size, 0, (const struct sockaddr *)send.GetAddress(), send.GetAddressLength());
DEBUG(net, 7, "[udp] sendto(%s)", send.GetAddressAsString()); DEBUG(net, 7, "[udp] sendto(%s)", NetworkAddressDumper().GetAddressAsString(&send));
/* Check for any errors, but ignore it otherwise */ /* Check for any errors, but ignore it otherwise */
if (res == -1) DEBUG(net, 1, "[udp] sendto(%s) failed with: %i", send.GetAddressAsString(), GET_LAST_ERROR()); if (res == -1) DEBUG(net, 1, "[udp] sendto(%s) failed with: %i", NetworkAddressDumper().GetAddressAsString(&send), GET_LAST_ERROR());
if (!all) break; if (!all) break;
} }
@@ -171,7 +171,7 @@ void NetworkUDPSocketHandler::ReceivePackets()
/* If the size does not match the packet must be corrupted. /* If the size does not match the packet must be corrupted.
* Otherwise it will be marked as corrupted later on. */ * Otherwise it will be marked as corrupted later on. */
if (nbytes != p.size) { if (nbytes != p.size) {
DEBUG(net, 1, "received a packet with mismatching size from %s, (%u, %u)", address.GetAddressAsString(), nbytes, p.size); DEBUG(net, 1, "received a packet with mismatching size from %s, (%u, %u)", NetworkAddressDumper().GetAddressAsString(&address), nbytes, p.size);
continue; continue;
} }
@@ -468,9 +468,9 @@ void NetworkUDPSocketHandler::HandleUDPPacket(Packet *p, NetworkAddress *client_
default: default:
if (this->HasClientQuit()) { if (this->HasClientQuit()) {
DEBUG(net, 0, "[udp] received invalid packet type %d from %s", type, client_addr->GetAddressAsString()); DEBUG(net, 0, "[udp] received invalid packet type %d from %s", type, NetworkAddressDumper().GetAddressAsString(client_addr));
} else { } else {
DEBUG(net, 0, "[udp] received illegal packet from %s", client_addr->GetAddressAsString()); DEBUG(net, 0, "[udp] received illegal packet from %s", NetworkAddressDumper().GetAddressAsString(client_addr));
} }
break; break;
} }
@@ -484,7 +484,7 @@ void NetworkUDPSocketHandler::Receive_EX_MULTI(Packet *p, NetworkAddress *client
uint16 payload_size = p->Recv_uint16(); uint16 payload_size = p->Recv_uint16();
DEBUG(net, 6, "[udp] received multi-part packet from %s: " OTTD_PRINTFHEX64 ", %u/%u, %u bytes", DEBUG(net, 6, "[udp] received multi-part packet from %s: " OTTD_PRINTFHEX64 ", %u/%u, %u bytes",
client_addr->GetAddressAsString(), token, index, total, payload_size); NetworkAddressDumper().GetAddressAsString(client_addr), token, index, total, payload_size);
if (total == 0 || index >= total) return; if (total == 0 || index >= total) return;
if (!p->CanReadFromPacket(payload_size)) return; if (!p->CanReadFromPacket(payload_size)) return;
@@ -502,7 +502,7 @@ void NetworkUDPSocketHandler::Receive_EX_MULTI(Packet *p, NetworkAddress *client
} }
DEBUG(net, 6, "[udp] merged multi-part packet from %s: " OTTD_PRINTFHEX64 ", %u bytes", DEBUG(net, 6, "[udp] merged multi-part packet from %s: " OTTD_PRINTFHEX64 ", %u bytes",
client_addr->GetAddressAsString(), token, total_payload); NetworkAddressDumper().GetAddressAsString(client_addr), token, total_payload);
Packet merged(this); Packet merged(this);
for (auto &frag : fs.fragments) { for (auto &frag : fs.fragments) {
@@ -514,7 +514,7 @@ void NetworkUDPSocketHandler::Receive_EX_MULTI(Packet *p, NetworkAddress *client
* Otherwise it will be marked as corrupted later on. */ * Otherwise it will be marked as corrupted later on. */
if (total_payload != merged.size) { if (total_payload != merged.size) {
DEBUG(net, 1, "received an extended packet with mismatching size from %s, (%u, %u)", DEBUG(net, 1, "received an extended packet with mismatching size from %s, (%u, %u)",
client_addr->GetAddressAsString(), total_payload, merged.size); NetworkAddressDumper().GetAddressAsString(client_addr), total_payload, merged.size);
} else { } else {
this->HandleUDPPacket(&merged, client_addr); this->HandleUDPPacket(&merged, client_addr);
} }
@@ -551,7 +551,7 @@ void NetworkUDPSocketHandler::Receive_EX_MULTI(Packet *p, NetworkAddress *client
*/ */
void NetworkUDPSocketHandler::ReceiveInvalidPacket(PacketUDPType type, NetworkAddress *client_addr) void NetworkUDPSocketHandler::ReceiveInvalidPacket(PacketUDPType type, NetworkAddress *client_addr)
{ {
DEBUG(net, 0, "[udp] received packet type %d on wrong port from %s", type, client_addr->GetAddressAsString()); DEBUG(net, 0, "[udp] received packet type %d on wrong port from %s", type, NetworkAddressDumper().GetAddressAsString(client_addr));
} }
void NetworkUDPSocketHandler::Receive_CLIENT_FIND_SERVER(Packet *p, NetworkAddress *client_addr) { this->ReceiveInvalidPacket(PACKET_UDP_CLIENT_FIND_SERVER, client_addr); } void NetworkUDPSocketHandler::Receive_CLIENT_FIND_SERVER(Packet *p, NetworkAddress *client_addr) { this->ReceiveInvalidPacket(PACKET_UDP_CLIENT_FIND_SERVER, client_addr); }

View File

@@ -659,7 +659,7 @@ void NetworkRebuildHostList()
_network_host_list.clear(); _network_host_list.clear();
for (NetworkGameList *item = _network_game_list; item != nullptr; item = item->next) { for (NetworkGameList *item = _network_game_list; item != nullptr; item = item->next) {
if (item->manually) _network_host_list.emplace_back(item->address.GetAddressAsString(false)); if (item->manually) _network_host_list.emplace_back(NetworkAddressDumper().GetAddressAsString(&(item->address), false));
} }
} }

View File

@@ -679,7 +679,9 @@ public:
DrawString(r.left + WD_FRAMERECT_LEFT, r.right - WD_FRAMERECT_RIGHT, y, STR_NETWORK_SERVER_LIST_SERVER_VERSION); // server version DrawString(r.left + WD_FRAMERECT_LEFT, r.right - WD_FRAMERECT_RIGHT, y, STR_NETWORK_SERVER_LIST_SERVER_VERSION); // server version
y += FONT_HEIGHT_NORMAL; y += FONT_HEIGHT_NORMAL;
SetDParamStr(0, sel->address.GetAddressAsString()); char network_addr_buffer[NETWORK_HOSTNAME_LENGTH + 6 + 7];
sel->address.GetAddressAsString(network_addr_buffer, lastof(network_addr_buffer));
SetDParamStr(0, network_addr_buffer);
DrawString(r.left + WD_FRAMERECT_LEFT, r.right - WD_FRAMERECT_RIGHT, y, STR_NETWORK_SERVER_LIST_SERVER_ADDRESS); // server address DrawString(r.left + WD_FRAMERECT_LEFT, r.right - WD_FRAMERECT_RIGHT, y, STR_NETWORK_SERVER_LIST_SERVER_ADDRESS); // server address
y += FONT_HEIGHT_NORMAL; y += FONT_HEIGHT_NORMAL;

View File

@@ -287,7 +287,7 @@ void ServerNetworkUDPSocketHandler::Receive_CLIENT_GET_NEWGRFS(Packet *p, Networ
size_t packet_len = 0; size_t packet_len = 0;
num_grfs = p->Recv_uint8 (); num_grfs = p->Recv_uint8 ();
DEBUG(net, 6, "[udp] newgrf data request (%u) from %s", num_grfs, client_addr->GetAddressAsString()); DEBUG(net, 6, "[udp] newgrf data request (%u) from %s", num_grfs, NetworkAddressDumper().GetAddressAsString(client_addr));
if (num_grfs > NETWORK_MAX_GRF_COUNT) return; if (num_grfs > NETWORK_MAX_GRF_COUNT) return;
@@ -306,7 +306,7 @@ void ServerNetworkUDPSocketHandler::Receive_CLIENT_GET_NEWGRFS(Packet *p, Networ
} }
this->SendPacket(&packet, client_addr); this->SendPacket(&packet, client_addr);
DEBUG(net, 6, "[udp] sent newgrf data response (%u of %u) to %s", (uint) in_reply.size(), num_grfs, client_addr->GetAddressAsString()); DEBUG(net, 6, "[udp] sent newgrf data response (%u of %u) to %s", (uint) in_reply.size(), num_grfs, NetworkAddressDumper().GetAddressAsString(client_addr));
packet_len = 0; packet_len = 0;
in_reply.clear(); in_reply.clear();
@@ -381,7 +381,7 @@ void ClientNetworkUDPSocketHandler::Receive_SERVER_RESPONSE_Common(Packet *p, Ne
/* Just a fail-safe.. should never happen */ /* Just a fail-safe.. should never happen */
if (_network_udp_server) return; if (_network_udp_server) return;
DEBUG(net, 4, "[udp]%s server response from %s", extended ? " extended" : "", client_addr->GetAddressAsString()); DEBUG(net, 4, "[udp]%s server response from %s", extended ? " extended" : "", NetworkAddressDumper().GetAddressAsString(client_addr));
/* Find next item */ /* Find next item */
item = NetworkGameListAddItem(*client_addr); item = NetworkGameListAddItem(*client_addr);
@@ -418,7 +418,7 @@ void ClientNetworkUDPSocketHandler::Receive_SERVER_RESPONSE_Common(Packet *p, Ne
this->SendPacket(&packet, &item->address); this->SendPacket(&packet, &item->address);
DEBUG(net, 4, "[udp] sent newgrf data request (%u) to %s", in_request_count, client_addr->GetAddressAsString()); DEBUG(net, 4, "[udp] sent newgrf data request (%u) to %s", in_request_count, NetworkAddressDumper().GetAddressAsString(client_addr));
in_request_count = 0; in_request_count = 0;
}; };
@@ -493,7 +493,7 @@ void ClientNetworkUDPSocketHandler::Receive_SERVER_NEWGRFS(Packet *p, NetworkAdd
uint i; uint i;
num_grfs = p->Recv_uint8 (); num_grfs = p->Recv_uint8 ();
DEBUG(net, 6, "[udp] newgrf data reply (%u) from %s", num_grfs, client_addr->GetAddressAsString()); DEBUG(net, 6, "[udp] newgrf data reply (%u) from %s", num_grfs, NetworkAddressDumper().GetAddressAsString(client_addr));
if (num_grfs > NETWORK_MAX_GRF_COUNT) return; if (num_grfs > NETWORK_MAX_GRF_COUNT) return;
@@ -570,7 +570,7 @@ void NetworkUDPQueryMasterServer()
_udp_client_socket->SendPacket(&p, &out_addr, true); _udp_client_socket->SendPacket(&p, &out_addr, true);
DEBUG(net, 2, "[udp] master server queried at %s", out_addr.GetAddressAsString()); DEBUG(net, 2, "[udp] master server queried at %s", NetworkAddressDumper().GetAddressAsString(&out_addr));
} }
/** Find all servers */ /** Find all servers */
@@ -634,8 +634,8 @@ static void NetworkUDPAdvertiseThread()
if (_session_key == 0 && session_key_retries++ == 2) { if (_session_key == 0 && session_key_retries++ == 2) {
DEBUG(net, 0, "[udp] advertising to the master server is failing"); DEBUG(net, 0, "[udp] advertising to the master server is failing");
DEBUG(net, 0, "[udp] we are not receiving the session key from the server"); DEBUG(net, 0, "[udp] we are not receiving the session key from the server");
DEBUG(net, 0, "[udp] please allow udp packets from %s to you to be delivered", out_addr.GetAddressAsString(false)); DEBUG(net, 0, "[udp] please allow udp packets from %s to you to be delivered", NetworkAddressDumper().GetAddressAsString(&out_addr, false));
DEBUG(net, 0, "[udp] please allow udp packets from you to %s to be delivered", out_addr.GetAddressAsString(false)); DEBUG(net, 0, "[udp] please allow udp packets from you to %s to be delivered", NetworkAddressDumper().GetAddressAsString(&out_addr, false));
} }
if (_session_key != 0 && _network_advertise_retries == 0) { if (_session_key != 0 && _network_advertise_retries == 0) {
DEBUG(net, 0, "[udp] advertising to the master server is failing"); DEBUG(net, 0, "[udp] advertising to the master server is failing");