diff --git a/src/command.cpp b/src/command.cpp index 9d240b762a..ac30e2972f 100644 --- a/src/command.cpp +++ b/src/command.cpp @@ -630,7 +630,7 @@ static int _docommand_recursive = 0; */ CommandCost DoCommand(const CommandContainer *container, DoCommandFlag flags) { - return DoCommand(container->tile, container->p1, container->p2, flags, container->cmd & CMD_ID_MASK, container->text); + return DoCommand(container->tile, container->p1, container->p2, flags, container->cmd & CMD_ID_MASK, container->text.c_str()); } /*! @@ -724,7 +724,7 @@ Money GetAvailableMoneyForCommand() */ bool DoCommandP(const CommandContainer *container, bool my_cmd) { - return DoCommandP(container->tile, container->p1, container->p2, container->cmd, container->callback, container->text, my_cmd); + return DoCommandP(container->tile, container->p1, container->p2, container->cmd, container->callback, container->text.c_str(), my_cmd); } /*! diff --git a/src/command_type.h b/src/command_type.h index 2fe957ef93..6237361e5e 100644 --- a/src/command_type.h +++ b/src/command_type.h @@ -15,6 +15,7 @@ #include "economy_type.h" #include "strings_type.h" #include "tile_type.h" +#include struct GRFFile; @@ -550,7 +551,7 @@ struct CommandContainer { uint32 cmd; ///< command being executed. CommandCallback *callback; ///< any callback function executed upon successful completion of the command. uint32 binary_length; ///< in case text contains binary data, this describes its length. - char text[MAX_CMD_TEXT_LENGTH]; ///< possible text sent for name changes etc, in bytes including '\0'. + std::string text; ///< possible text sent for name changes etc. }; #endif /* COMMAND_TYPE_H */ diff --git a/src/network/core/packet.cpp b/src/network/core/packet.cpp index a8a82823e5..be922428d7 100644 --- a/src/network/core/packet.cpp +++ b/src/network/core/packet.cpp @@ -330,10 +330,26 @@ void Packet::Recv_string(char *buffer, size_t size, StringValidationSettings set str_validate(bufp, last, settings); } +/** + * Reads a string till it finds a '\0' in the stream. + * @param buffer The buffer to put the data into. + * @param settings The string validation settings. + */ +void Packet::Recv_string(std::string &buffer, StringValidationSettings settings) +{ + /* Don't allow reading from a closed socket */ + if (cs->HasClientQuit()) return; + + size_t length = ttd_strnlen((const char *)(this->buffer + this->pos), this->size - this->pos - 1); + buffer.assign((const char *)(this->buffer + this->pos), length); + this->pos += length + 1; + str_validate(const_cast(buffer.c_str()), buffer.c_str() + buffer.size(), settings); +} + /** * Reads binary data. * @param buffer The buffer to put the data into. - * @param size The size of the buffer. + * @param size The size of the data. */ void Packet::Recv_binary(char *buffer, size_t size) { @@ -344,4 +360,18 @@ void Packet::Recv_binary(char *buffer, size_t size) this->pos += (PacketSize) size; } +/** + * Reads binary data. + * @param buffer The buffer to put the data into. + * @param size The size of the data. + */ +void Packet::Recv_binary(std::string &buffer, size_t size) +{ + /* Don't allow reading from a closed socket */ + if (cs->HasClientQuit()) return; + + buffer.assign((const char *) &this->buffer[this->pos], size); + this->pos += (PacketSize) size; +} + #endif /* ENABLE_NETWORK */ diff --git a/src/network/core/packet.h b/src/network/core/packet.h index ec24ea06bc..185969dc35 100644 --- a/src/network/core/packet.h +++ b/src/network/core/packet.h @@ -17,6 +17,7 @@ #include "config.h" #include "core.h" #include "../../string_type.h" +#include #ifdef ENABLE_NETWORK @@ -88,7 +89,9 @@ public: uint32 Recv_uint32(); uint64 Recv_uint64(); void Recv_string(char *buffer, size_t size, StringValidationSettings settings = SVS_REPLACE_WITH_QUESTION_MARK); + void Recv_string(std::string &buffer, StringValidationSettings settings = SVS_REPLACE_WITH_QUESTION_MARK); void Recv_binary(char *buffer, size_t size); + void Recv_binary(std::string &buffer, size_t size); }; #endif /* ENABLE_NETWORK */ diff --git a/src/network/core/tcp_game.h b/src/network/core/tcp_game.h index 5c6cb5c34d..f929997d51 100644 --- a/src/network/core/tcp_game.h +++ b/src/network/core/tcp_game.h @@ -18,6 +18,7 @@ #include "tcp.h" #include "../network_type.h" #include "../../core/pool_type.hpp" +#include #ifdef ENABLE_NETWORK @@ -135,13 +136,16 @@ class CommandQueue { CommandPacket *last; ///< The last packet in the queue; only valid when first != NULL. uint count; ///< The number of items in the queue. + void Append(CommandPacket *p, bool move); + public: /** Initialise the command queue. */ CommandQueue() : first(NULL), last(NULL), count(0) {} /** Clear the command queue. */ ~CommandQueue() { this->Free(); } - void Append(CommandPacket *p); - CommandPacket *Pop(bool ignore_paused = false); + void Append(CommandPacket &p) { this->Append(&p, false); } + void Append(CommandPacket &&p) { this->Append(&p, true); } + std::unique_ptr Pop(bool ignore_paused = false); CommandPacket *Peek(bool ignore_paused = false); void Free(); /** Get the number of items in the queue. */ diff --git a/src/network/network.cpp b/src/network/network.cpp index e935ef4b5e..29348f5470 100644 --- a/src/network/network.cpp +++ b/src/network/network.cpp @@ -906,7 +906,7 @@ void NetworkGameLoop() static Date next_date = 0; static uint32 next_date_fract; static uint8 next_tick_skip_counter; - static CommandPacket *cp = NULL; + static std::unique_ptr cp; static bool check_sync_state = false; static uint32 sync_state[2]; if (f == NULL && next_date == 0) { @@ -917,10 +917,9 @@ void NetworkGameLoop() while (f != NULL && !feof(f)) { if (_date == next_date && _date_fract == next_date_fract) { if (cp != NULL) { - NetworkSendCommand(cp->tile, cp->p1, cp->p2, cp->cmd & ~CMD_FLAGS_MASK, NULL, cp->text, cp->company); - DEBUG(net, 0, "injecting: date{%08x; %02x; %02x}; %02x; %06x; %08x; %08x; %08x; \"%s\" (%s)", _date, _date_fract, _tick_skip_counter, (int)_current_company, cp->tile, cp->p1, cp->p2, cp->cmd, cp->text, GetCommandName(cp->cmd)); - free(cp); - cp = NULL; + NetworkSendCommand(cp->tile, cp->p1, cp->p2, cp->cmd & ~CMD_FLAGS_MASK, NULL, cp->text.c_str(), cp->company, cp->binary_length); + DEBUG(net, 0, "injecting: date{%08x; %02x; %02x}; %02x; %06x; %08x; %08x; %08x; \"%s\" (%x) (%s)", _date, _date_fract, _tick_skip_counter, (int)_current_company, cp->tile, cp->p1, cp->p2, cp->cmd, cp->text.c_str(), cp->binary_length, GetCommandName(cp->cmd)); + cp.reset(); } if (check_sync_state) { if (sync_state[0] == _random.state[0] && sync_state[1] == _random.state[1]) { @@ -954,24 +953,29 @@ void NetworkGameLoop() ) { p += 5; if (*p == ' ') p++; - cp = CallocT(1); + cp.reset(new CommandPacket()); int company; - int ret = sscanf(p, "date{%x; %x; %x}; %x; %x; %x; %x; %x; \"%[^\"]\"", &next_date, &next_date_fract, &next_tick_skip_counter, &company, &cp->tile, &cp->p1, &cp->p2, &cp->cmd, cp->text); + cp->text.resize(MAX_CMD_TEXT_LENGTH); + int ret = sscanf(p, "date{%x; %x; %x}; %x; %x; %x; %x; %x; \"%[^\"]\"", &next_date, &next_date_fract, &next_tick_skip_counter, &company, &cp->tile, &cp->p1, &cp->p2, &cp->cmd, const_cast(cp->text.c_str())); /* There are 9 pieces of data to read, however the last is a * string that might or might not exist. Ignore it if that * string misses because in 99% of the time it's not used. */ assert(ret == 9 || ret == 8); cp->company = (CompanyID)company; + cp->binary_length = 0; } else if (strncmp(p, "join: ", 6) == 0) { /* Manually insert a pause when joining; this way the client can join at the exact right time. */ int ret = sscanf(p + 6, "date{%x; %x; %x}", &next_date, &next_date_fract, &next_tick_skip_counter); assert(ret == 3); DEBUG(net, 0, "injecting pause for join at date{%08x; %02x; %02x}; please join when paused", next_date, next_date_fract, next_tick_skip_counter); - cp = CallocT(1); + cp.reset(new CommandPacket()); + cp->tile = 0; cp->company = COMPANY_SPECTATOR; cp->cmd = CMD_PAUSE; cp->p1 = PM_PAUSED_NORMAL; cp->p2 = 1; + cp->callback = NULL; + cp->binary_length = 0; _ddc_fastforward = false; } else if (strncmp(p, "sync: ", 6) == 0) { int ret = sscanf(p + 6, "date{%x; %x; %x}; %x; %x", &next_date, &next_date_fract, &next_tick_skip_counter, &sync_state[0], &sync_state[1]); diff --git a/src/network/network_admin.cpp b/src/network/network_admin.cpp index 4facb668ae..c5c14d0fc3 100644 --- a/src/network/network_admin.cpp +++ b/src/network/network_admin.cpp @@ -655,7 +655,7 @@ NetworkRecvStatus ServerNetworkAdminSocketHandler::SendCmdLogging(ClientID clien p->Send_uint32(cp->p1); p->Send_uint32(cp->p2); p->Send_uint32(cp->tile); - p->Send_string(cp->text); + p->Send_string(cp->text.c_str()); p->Send_uint32(cp->frame); this->SendPacket(p); diff --git a/src/network/network_client.cpp b/src/network/network_client.cpp index 3ae4f7b4f5..1ea259809a 100644 --- a/src/network/network_client.cpp +++ b/src/network/network_client.cpp @@ -955,7 +955,7 @@ NetworkRecvStatus ClientNetworkGameSocketHandler::Receive_SERVER_COMMAND(Packet return NETWORK_RECV_STATUS_MALFORMED_PACKET; } - this->incoming_queue.Append(&cp); + this->incoming_queue.Append(std::move(cp)); return NETWORK_RECV_STATUS_OKAY; } diff --git a/src/network/network_command.cpp b/src/network/network_command.cpp index 9920fe137c..42bf85ab31 100644 --- a/src/network/network_command.cpp +++ b/src/network/network_command.cpp @@ -63,10 +63,14 @@ static CommandCallback * const _callback_table[] = { * @param p The packet to append to the queue. * @note A new instance of the CommandPacket will be made. */ -void CommandQueue::Append(CommandPacket *p) +void CommandQueue::Append(CommandPacket *p, bool move) { - CommandPacket *add = MallocT(1); - *add = *p; + CommandPacket *add = new CommandPacket(); + if (move) { + *add = std::move(*p); + } else { + *add = *p; + } add->next = NULL; if (this->first == NULL) { this->first = add; @@ -82,8 +86,9 @@ void CommandQueue::Append(CommandPacket *p) * @param ignore_paused Whether to ignore commands that may not be executed while paused. * @return the first item in the queue. */ -CommandPacket *CommandQueue::Pop(bool ignore_paused) +std::unique_ptr CommandQueue::Pop(bool ignore_paused) { + CommandPacket **prev = &this->first; CommandPacket *ret = this->first; CommandPacket *prev_item = NULL; @@ -99,7 +104,7 @@ CommandPacket *CommandQueue::Pop(bool ignore_paused) *prev = ret->next; this->count--; } - return ret; + return std::unique_ptr(ret); } /** @@ -120,10 +125,8 @@ CommandPacket *CommandQueue::Peek(bool ignore_paused) /** Free everything that is in the queue. */ void CommandQueue::Free() { - CommandPacket *cp; - while ((cp = this->Pop()) != NULL) { - free(cp); - } + std::unique_ptr cp; + while ((cp = this->Pop()) != NULL) {} assert(this->count == 0); } @@ -157,9 +160,13 @@ void NetworkSendCommand(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd, Comman c.binary_length = binary_length; if (binary_length == 0) { - strecpy(c.text, (text != NULL) ? text : "", lastof(c.text)); + if (text != NULL) { + c.text.assign(text); + } else { + c.text.clear(); + } } else { - memcpy(c.text, text, binary_length); + c.text.assign(text, binary_length); } if (_network_server) { @@ -172,7 +179,7 @@ void NetworkSendCommand(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd, Comman c.frame = _frame_counter_max + 1; c.my_cmd = true; - _local_wait_queue.Append(&c); + _local_wait_queue.Append(std::move(c)); return; } @@ -196,7 +203,7 @@ void NetworkSyncCommandQueue(NetworkClientSocket *cs) for (CommandPacket *p = _local_execution_queue.Peek(); p != NULL; p = p->next) { CommandPacket c = *p; c.callback = 0; - cs->outgoing_queue.Append(&c); + cs->outgoing_queue.Append(std::move(c)); } } @@ -227,7 +234,6 @@ void NetworkExecuteLocalCommandQueue() DoCommandP(cp, cp->my_cmd); queue.Pop(); - free(cp); } /* Local company may have changed, so we should not restore the old value */ @@ -260,13 +266,13 @@ static void DistributeCommandPacket(CommandPacket &cp, const NetworkClientSocket * first place. This filters that out. */ cp.callback = (cs != owner) ? NULL : callback; cp.my_cmd = (cs == owner); - cs->outgoing_queue.Append(&cp); + cs->outgoing_queue.Append(cp); } } cp.callback = (cs != owner) ? NULL : callback; cp.my_cmd = (cs == owner); - _local_execution_queue.Append(&cp); + _local_execution_queue.Append(cp); } /** @@ -283,11 +289,10 @@ static void DistributeQueue(CommandQueue *queue, const NetworkClientSocket *owne int to_go = _settings_client.network.commands_per_frame; #endif - CommandPacket *cp; + std::unique_ptr cp; while (--to_go >= 0 && (cp = queue->Pop(true)) != NULL) { DistributeCommandPacket(*cp, owner); - NetworkAdminCmdLogging(owner, cp); - free(cp); + NetworkAdminCmdLogging(owner, cp.get()); } } @@ -323,9 +328,10 @@ const char *NetworkGameSocketHandler::ReceiveCommand(Packet *p, CommandPacket *c cp->tile = p->Recv_uint32(); cp->binary_length = p->Recv_uint32(); if (cp->binary_length == 0) { - p->Recv_string(cp->text, lengthof(cp->text), (!_network_server && GetCommandFlags(cp->cmd) & CMD_STR_CTRL) != 0 ? SVS_ALLOW_CONTROL_CODE | SVS_REPLACE_WITH_QUESTION_MARK : SVS_REPLACE_WITH_QUESTION_MARK); + p->Recv_string(cp->text, (!_network_server && GetCommandFlags(cp->cmd) & CMD_STR_CTRL) != 0 ? SVS_ALLOW_CONTROL_CODE | SVS_REPLACE_WITH_QUESTION_MARK : SVS_REPLACE_WITH_QUESTION_MARK); } else { if ((p->pos + (PacketSize) cp->binary_length + /* callback index */ 1) > p->size) return "invalid binary data length"; + if (cp->binary_length > MAX_CMD_TEXT_LENGTH) return "over-size binary data length"; p->Recv_binary(cp->text, cp->binary_length); } @@ -350,9 +356,10 @@ void NetworkGameSocketHandler::SendCommand(Packet *p, const CommandPacket *cp) p->Send_uint32(cp->tile); p->Send_uint32(cp->binary_length); if (cp->binary_length == 0) { - p->Send_string(cp->text); + p->Send_string(cp->text.c_str()); } else { - p->Send_binary(cp->text, cp->binary_length); + assert(cp->text.size() >= cp->binary_length); + p->Send_binary(cp->text.c_str(), cp->binary_length); } byte callback = 0; diff --git a/src/network/network_server.cpp b/src/network/network_server.cpp index a5d0afdf76..747a7ad67f 100644 --- a/src/network/network_server.cpp +++ b/src/network/network_server.cpp @@ -1144,7 +1144,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_COMMAND(Packet if (GetCommandFlags(cp.cmd) & CMD_CLIENT_ID) cp.p2 = this->client_id; - this->incoming_queue.Append(&cp); + this->incoming_queue.Append(std::move(cp)); return NETWORK_RECV_STATUS_OKAY; } @@ -1799,10 +1799,9 @@ void NetworkServerSetCompanyPassword(CompanyID company_id, const char *password, */ static void NetworkHandleCommandQueue(NetworkClientSocket *cs) { - CommandPacket *cp; + std::unique_ptr cp; while ((cp = cs->outgoing_queue.Pop()) != NULL) { - cs->SendCommand(cp); - free(cp); + cs->SendCommand(cp.get()); } }