From 3dfd4f9ce2c46a40e5bd5fca412f49487934823e Mon Sep 17 00:00:00 2001 From: Simon Newton Date: Fri, 2 Jan 2015 18:28:00 -0800 Subject: [PATCH 1/6] Address some more defects. --- common/network/TCPConnector.cpp | 4 +++- common/network/TCPSocket.cpp | 2 ++ common/rdm/AckTimerResponder.cpp | 3 +++ plugins/e131/e131/E131PDU.cpp | 8 ++++---- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/common/network/TCPConnector.cpp b/common/network/TCPConnector.cpp index d3e54ca814..cfd25fc8c3 100644 --- a/common/network/TCPConnector.cpp +++ b/common/network/TCPConnector.cpp @@ -150,7 +150,9 @@ TCPConnector::TCPConnectionID TCPConnector::Connect( return 0; } } else { - // connect returned immediately + // Connect returned immediately + // The callback takes ownership of the socket descriptor. + // coverity(RESOURCE_LEAK) callback->Run(sd, 0); return 0; } diff --git a/common/network/TCPSocket.cpp b/common/network/TCPSocket.cpp index 7cb0640fcc..d34b05c3f6 100644 --- a/common/network/TCPSocket.cpp +++ b/common/network/TCPSocket.cpp @@ -303,6 +303,8 @@ void TCPAcceptingSocket::PerformRead() { } if (m_factory) { + // The callback takes ownership of the new socket descriptor + // coverity(RESOURCE_LEAK) m_factory->NewTCPSocket(sd); } else { OLA_WARN << "Accepted new TCP Connection but no factory registered"; diff --git a/common/rdm/AckTimerResponder.cpp b/common/rdm/AckTimerResponder.cpp index e346566687..0e5777485e 100644 --- a/common/rdm/AckTimerResponder.cpp +++ b/common/rdm/AckTimerResponder.cpp @@ -220,6 +220,7 @@ const RDMResponse *AckTimerResponder::ResponseFromQueuedMessage( queued_response->ParamDataSize()); break; case RDMCommand::SET_COMMAND_RESPONSE: + // coverity(SWAPPED_ARGUMENTS) return new RDMSetResponse( request->DestinationUID(), request->SourceUID(), @@ -352,6 +353,7 @@ const RDMResponse *AckTimerResponder::SetDmxStartAddress( uint16_t ack_time = 1 + ACK_TIMER_MS / 100; ack_time = HostToNetwork(ack_time); + // coverity(SWAPPED_ARGUMENTS) return new RDMSetResponse( request->DestinationUID(), request->SourceUID(), @@ -402,6 +404,7 @@ const RDMResponse *AckTimerResponder::SetIdentify( uint16_t ack_time = 1 + ACK_TIMER_MS / 100; ack_time = HostToNetwork(ack_time); + // coverity(SWAPPED_ARGUMENTS) return new RDMSetResponse( request->DestinationUID(), request->SourceUID(), diff --git a/plugins/e131/e131/E131PDU.cpp b/plugins/e131/e131/E131PDU.cpp index 23ba1ac848..9ab7580bf5 100644 --- a/plugins/e131/e131/E131PDU.cpp +++ b/plugins/e131/e131/E131PDU.cpp @@ -118,8 +118,8 @@ bool E131PDU::PackData(uint8_t *data, unsigned int *length) const { void E131PDU::PackHeader(OutputStream *stream) const { if (m_header.UsingRev2()) { E131Rev2Header::e131_rev2_pdu_header header; - strncpy(header.source, m_header.Source().data(), - E131Rev2Header::REV2_SOURCE_NAME_LEN); + strings::CopyToFixedLengthBuffer(m_header.Source(), header.source, + arraysize(header.source)); header.priority = m_header.Priority(); header.sequence = m_header.Sequence(); header.universe = HostToNetwork(m_header.Universe()); @@ -127,8 +127,8 @@ void E131PDU::PackHeader(OutputStream *stream) const { sizeof(E131Rev2Header::e131_rev2_pdu_header)); } else { E131Header::e131_pdu_header header; - strncpy(header.source, m_header.Source().data(), - E131Header::SOURCE_NAME_LEN); + strings::CopyToFixedLengthBuffer(m_header.Source(), header.source, + arraysize(header.source)); header.priority = m_header.Priority(); header.reserved = 0; header.sequence = m_header.Sequence(); From eda1ee85746c2bc750a0fdf9008126f8418b60c5 Mon Sep 17 00:00:00 2001 From: Simon Newton Date: Fri, 2 Jan 2015 19:25:45 -0800 Subject: [PATCH 2/6] Fix some more defects. --- common/network/Interface.cpp | 5 +- common/network/PosixInterfacePicker.cpp | 4 ++ common/rdm/MessageDeserializer.cpp | 4 +- common/rdm/MovingLightResponder.cpp | 2 + common/rdm/RDMCommand.cpp | 5 ++ common/rdm/ResponderHelper.cpp | 1 + common/rdm/StringMessageBuilder.cpp | 1 + examples/ola-rdm.cpp | 14 ++++-- include/ola/rdm/RDMMessagePrinters.h | 5 +- include/ola/web/JsonPatchParser.h | 5 +- plugins/artnet/ArtNetNode.cpp | 6 ++- plugins/e131/e131/E131TestFramework.cpp | 14 ++++++ plugins/e131/e131/E131TestFramework.h | 12 +---- plugins/spi/SPIWriter.cpp | 4 +- tools/e133/E133Device.cpp | 66 ++++++++++++------------- tools/e133/EndpointManager.h | 5 +- tools/rdmpro/rdm-sniffer.cpp | 1 + 17 files changed, 99 insertions(+), 55 deletions(-) diff --git a/common/network/Interface.cpp b/common/network/Interface.cpp index 380cb56966..01094f51c5 100644 --- a/common/network/Interface.cpp +++ b/common/network/Interface.cpp @@ -134,7 +134,10 @@ InterfaceBuilder::InterfaceBuilder() : m_ip_address(0), m_broadcast_address(0), m_subnet_mask(0), - m_hw_address() { + m_hw_address(), + m_loopback(false), + m_index(Interface::DEFAULT_INDEX), + m_type(Interface::ARP_VOID_TYPE) { } diff --git a/common/network/PosixInterfacePicker.cpp b/common/network/PosixInterfacePicker.cpp index 0b6e798559..801710267e 100644 --- a/common/network/PosixInterfacePicker.cpp +++ b/common/network/PosixInterfacePicker.cpp @@ -160,6 +160,8 @@ vector PosixInterfacePicker::GetInterfaces( } } +#ifdef HAVE_SOCKADDR_DL_STRUCT + // The only way hwaddr is non-null is if HAVE_SOCKADDR_DL_STRUCT is defined. if ((interface.name == last_dl_iface_name) && hwaddr) { if (hwlen == MACAddress::LENGTH) { interface.hw_address = MACAddress(reinterpret_cast(hwaddr)); @@ -169,6 +171,8 @@ vector PosixInterfacePicker::GetInterfaces( << ", expecting " << MACAddress::LENGTH; } } +#endif + struct sockaddr_in *sin = (struct sockaddr_in *) &iface->ifr_addr; interface.ip_address = IPV4Address(sin->sin_addr.s_addr); diff --git a/common/rdm/MessageDeserializer.cpp b/common/rdm/MessageDeserializer.cpp index a968b418fe..8645236d02 100644 --- a/common/rdm/MessageDeserializer.cpp +++ b/common/rdm/MessageDeserializer.cpp @@ -38,7 +38,9 @@ using std::vector; MessageDeserializer::MessageDeserializer() : m_data(NULL), m_length(0), - m_offset(0) { + m_offset(0), + m_variable_field_size(0), + m_insufficient_data(false) { } diff --git a/common/rdm/MovingLightResponder.cpp b/common/rdm/MovingLightResponder.cpp index 3323e101da..373a22e6d1 100644 --- a/common/rdm/MovingLightResponder.cpp +++ b/common/rdm/MovingLightResponder.cpp @@ -327,6 +327,7 @@ const RDMResponse *MovingLightResponder::GetLanguageCapabilities( 'f', 'r', 'd', 'e', }; + // coverity(SWAPPED_ARGUMENTS) return new RDMGetResponse( request->DestinationUID(), request->SourceUID(), @@ -345,6 +346,7 @@ const RDMResponse *MovingLightResponder::GetLanguage( return NackWithReason(request, NR_FORMAT_ERROR); } + // coverity(SWAPPED_ARGUMENTS) return new RDMGetResponse( request->DestinationUID(), request->SourceUID(), diff --git a/common/rdm/RDMCommand.cpp b/common/rdm/RDMCommand.cpp index 0c18c3e10c..086f606e81 100644 --- a/common/rdm/RDMCommand.cpp +++ b/common/rdm/RDMCommand.cpp @@ -681,6 +681,7 @@ RDMResponse *NackWithReason(const RDMRequest *request, uint16_t reason = ola::network::HostToNetwork(static_cast( reason_enum)); if (request->CommandClass() == ola::rdm::RDMCommand::GET_COMMAND) { + // coverity(SWAPPED_ARGUMENTS) return new ola::rdm::RDMGetResponse( request->DestinationUID(), request->SourceUID(), @@ -692,6 +693,7 @@ RDMResponse *NackWithReason(const RDMRequest *request, reinterpret_cast(&reason), sizeof(reason)); } else { + // coverity(SWAPPED_ARGUMENTS) return new ola::rdm::RDMSetResponse( request->DestinationUID(), request->SourceUID(), @@ -734,6 +736,7 @@ RDMResponse *GetResponseWithPid(const RDMRequest *request, uint8_t outstanding_messages) { switch (request->CommandClass()) { case RDMCommand::GET_COMMAND: + // coverity(SWAPPED_ARGUMENTS) return new RDMGetResponse( request->DestinationUID(), request->SourceUID(), @@ -745,6 +748,7 @@ RDMResponse *GetResponseWithPid(const RDMRequest *request, data, length); case RDMCommand::SET_COMMAND: + // coverity(SWAPPED_ARGUMENTS) return new RDMSetResponse( request->DestinationUID(), request->SourceUID(), @@ -756,6 +760,7 @@ RDMResponse *GetResponseWithPid(const RDMRequest *request, data, length); case RDMCommand::DISCOVER_COMMAND: + // coverity(SWAPPED_ARGUMENTS) return new RDMDiscoveryResponse( request->DestinationUID(), request->SourceUID(), diff --git a/common/rdm/ResponderHelper.cpp b/common/rdm/ResponderHelper.cpp index ea06c73431..2d5bf36395 100644 --- a/common/rdm/ResponderHelper.cpp +++ b/common/rdm/ResponderHelper.cpp @@ -1042,6 +1042,7 @@ const RDMResponse *ResponderHelper::EmptyGetResponse( const RDMResponse *ResponderHelper::EmptySetResponse( const RDMRequest *request, uint8_t queued_message_count) { + // coverity(SWAPPED_ARGUMENTS) return new RDMSetResponse( request->DestinationUID(), request->SourceUID(), diff --git a/common/rdm/StringMessageBuilder.cpp b/common/rdm/StringMessageBuilder.cpp index e7d3462296..c528475d4b 100644 --- a/common/rdm/StringMessageBuilder.cpp +++ b/common/rdm/StringMessageBuilder.cpp @@ -46,6 +46,7 @@ using std::vector; StringMessageBuilder::StringMessageBuilder() : m_offset(0), m_input_size(0), + m_group_instance_count(0), m_error(false) { } diff --git a/examples/ola-rdm.cpp b/examples/ola-rdm.cpp index 26d2fb43f9..ed21bcee95 100644 --- a/examples/ola-rdm.cpp +++ b/examples/ola-rdm.cpp @@ -223,16 +223,24 @@ class RDMController { const string &rdm_data); private: - typedef struct { + struct PendingRequest { + public: + PendingRequest() + : universe(0), + uid(NULL), + sub_device(0), + pid_value(0) { + } + unsigned int universe; const UID *uid; uint16_t sub_device; uint16_t pid_value; - } pending_request_t; + }; ola::OlaCallbackClientWrapper m_ola_client; PidStoreHelper m_pid_helper; - pending_request_t m_pending_request; + PendingRequest m_pending_request; void FetchQueuedMessage(); void PrintRemainingMessages(uint8_t message_count); diff --git a/include/ola/rdm/RDMMessagePrinters.h b/include/ola/rdm/RDMMessagePrinters.h index 211a15fb65..966f16be60 100644 --- a/include/ola/rdm/RDMMessagePrinters.h +++ b/include/ola/rdm/RDMMessagePrinters.h @@ -273,7 +273,10 @@ class LanguageCapabilityPrinter: public ola::messaging::MessagePrinter { */ class ClockPrinter: public ola::messaging::MessagePrinter { public: - ClockPrinter() : ola::messaging::MessagePrinter(), m_offset(0) {} + ClockPrinter() + : ola::messaging::MessagePrinter(), + m_year(0), + m_offset(0) {} void Visit(const ola::messaging::UInt16MessageField *message) { m_year = message->Value(); } diff --git a/include/ola/web/JsonPatchParser.h b/include/ola/web/JsonPatchParser.h index 1e519dbf7b..a14b76c1d6 100644 --- a/include/ola/web/JsonPatchParser.h +++ b/include/ola/web/JsonPatchParser.h @@ -54,7 +54,10 @@ class JsonPatchParser : public JsonParserInterface { public: explicit JsonPatchParser(JsonPatchSet *patch_set) : JsonParserInterface(), - m_patch_set(patch_set) {} + m_patch_set(patch_set), + m_parser_depth(0), + m_state(TOP) { + } void Begin(); void End(); diff --git a/plugins/artnet/ArtNetNode.cpp b/plugins/artnet/ArtNetNode.cpp index 9728764522..91ed51ad5a 100644 --- a/plugins/artnet/ArtNetNode.cpp +++ b/plugins/artnet/ArtNetNode.cpp @@ -405,9 +405,11 @@ uint8_t ArtNetNodeImpl::GetInputPortUniverse(uint8_t port_id) const { void ArtNetNodeImpl::DisableInputPort(uint8_t port_id) { InputPort *port = GetInputPort(port_id); - bool was_enabled = port->enabled; - if (port) + bool was_enabled = false; + if (port) { + was_enabled = port->enabled; port->enabled = false; + } if (was_enabled) SendPollReplyIfRequired(); diff --git a/plugins/e131/e131/E131TestFramework.cpp b/plugins/e131/e131/E131TestFramework.cpp index a820279596..389e531631 100644 --- a/plugins/e131/e131/E131TestFramework.cpp +++ b/plugins/e131/e131/E131TestFramework.cpp @@ -46,6 +46,20 @@ using std::endl; using std::string; using std::vector; +StateManager::StateManager(const std::vector &states, + bool interactive_mode) + : m_interactive(interactive_mode), + m_count(0), + m_ticker(0), + m_local_node(NULL), + m_node1(NULL), + m_node2(NULL), + m_ss(NULL), + m_states(states), + m_stdin_descriptor(STDIN_FILENO) { + memset(&m_old_tc, 0, sizeof(m_old_tc)); +} + bool StateManager::Init() { m_cid1 = CID::Generate(); m_cid2 = CID::Generate(); diff --git a/plugins/e131/e131/E131TestFramework.h b/plugins/e131/e131/E131TestFramework.h index 8c8705ab8d..45a22c7eb7 100644 --- a/plugins/e131/e131/E131TestFramework.h +++ b/plugins/e131/e131/E131TestFramework.h @@ -304,17 +304,7 @@ class NodeVarySequenceNumber: public NodeAction { class StateManager { public: StateManager(const std::vector &states, - bool interactive_mode = false): - m_interactive(interactive_mode), - m_count(0), - m_ticker(0), - m_local_node(NULL), - m_node1(NULL), - m_node2(NULL), - m_ss(NULL), - m_states(states), - m_stdin_descriptor(STDIN_FILENO) { - } + bool interactive_mode = false); ~StateManager(); bool Init(); void Run() { m_ss->Run(); } diff --git a/plugins/spi/SPIWriter.cpp b/plugins/spi/SPIWriter.cpp index 1927159a15..36b66c5423 100644 --- a/plugins/spi/SPIWriter.cpp +++ b/plugins/spi/SPIWriter.cpp @@ -52,7 +52,9 @@ SPIWriter::SPIWriter(const string &spi_device, : m_device_path(spi_device), m_spi_speed(options.spi_speed), m_cs_enable_high(options.cs_enable_high), - m_fd(-1) { + m_fd(-1), + m_error_map_var(NULL), + m_write_map_var(NULL) { OLA_INFO << "Created SPI Writer " << spi_device << " with speed " << options.spi_speed << ", CE is " << m_cs_enable_high; if (export_map) { diff --git a/tools/e133/E133Device.cpp b/tools/e133/E133Device.cpp index 76ef3f1dd9..e3f85fa5b5 100644 --- a/tools/e133/E133Device.cpp +++ b/tools/e133/E133Device.cpp @@ -56,6 +56,37 @@ using std::auto_ptr; using std::string; using std::vector; +// TODO(simon): At some point move this to a common E1.33 library. +ola::e133::E133StatusCode RDMResponseCodeToE133Status( + ola::rdm::rdm_response_code response_code) { + switch (response_code) { + case ola::rdm::RDM_COMPLETED_OK: + return ola::e133::SC_E133_ACK; + case ola::rdm::RDM_WAS_BROADCAST: + return ola::e133::SC_E133_BROADCAST_COMPLETE; + case ola::rdm::RDM_FAILED_TO_SEND: + case ola::rdm::RDM_TIMEOUT: + return ola::e133::SC_E133_RDM_TIMEOUT; + case ola::rdm::RDM_UNKNOWN_UID: + return ola::e133::SC_E133_UNKNOWN_UID; + case ola::rdm::RDM_INVALID_RESPONSE: + case ola::rdm::RDM_CHECKSUM_INCORRECT: + case ola::rdm::RDM_TRANSACTION_MISMATCH: + case ola::rdm::RDM_SUB_DEVICE_MISMATCH: + case ola::rdm::RDM_SRC_UID_MISMATCH: + case ola::rdm::RDM_DEST_UID_MISMATCH: + case ola::rdm::RDM_WRONG_SUB_START_CODE: + case ola::rdm::RDM_PACKET_TOO_SHORT: + case ola::rdm::RDM_PACKET_LENGTH_MISMATCH: + case ola::rdm::RDM_PARAM_LENGTH_MISMATCH: + case ola::rdm::RDM_INVALID_COMMAND_CLASS: + case ola::rdm::RDM_COMMAND_CLASS_MISMATCH: + case ola::rdm::RDM_INVALID_RESPONSE_TYPE: + case ola::rdm::RDM_PLUGIN_DISCOVERY_NOT_SUPPORTED: + case ola::rdm::RDM_DUB_RESPONSE: + return ola::e133::SC_E133_RDM_INVALID_RESPONSE; + } +} E133Device::E133Device(ola::io::SelectServerInterface *ss, const ola::acn::CID &cid, @@ -227,40 +258,9 @@ void E133Device::EndpointRequestComplete( auto_ptr response(response_ptr); if (response_code != ola::rdm::RDM_COMPLETED_OK) { - ola::e133::E133StatusCode status_code = - ola::e133::SC_E133_RDM_INVALID_RESPONSE; string description = ola::rdm::ResponseCodeToString(response_code); - switch (response_code) { - case ola::rdm::RDM_COMPLETED_OK: - break; - case ola::rdm::RDM_WAS_BROADCAST: - status_code = ola::e133::SC_E133_BROADCAST_COMPLETE; - break; - case ola::rdm::RDM_FAILED_TO_SEND: - case ola::rdm::RDM_TIMEOUT: - status_code = ola::e133::SC_E133_RDM_TIMEOUT; - break; - case ola::rdm::RDM_UNKNOWN_UID: - status_code = ola::e133::SC_E133_UNKNOWN_UID; - break; - case ola::rdm::RDM_INVALID_RESPONSE: - case ola::rdm::RDM_CHECKSUM_INCORRECT: - case ola::rdm::RDM_TRANSACTION_MISMATCH: - case ola::rdm::RDM_SUB_DEVICE_MISMATCH: - case ola::rdm::RDM_SRC_UID_MISMATCH: - case ola::rdm::RDM_DEST_UID_MISMATCH: - case ola::rdm::RDM_WRONG_SUB_START_CODE: - case ola::rdm::RDM_PACKET_TOO_SHORT: - case ola::rdm::RDM_PACKET_LENGTH_MISMATCH: - case ola::rdm::RDM_PARAM_LENGTH_MISMATCH: - case ola::rdm::RDM_INVALID_COMMAND_CLASS: - case ola::rdm::RDM_COMMAND_CLASS_MISMATCH: - case ola::rdm::RDM_INVALID_RESPONSE_TYPE: - case ola::rdm::RDM_PLUGIN_DISCOVERY_NOT_SUPPORTED: - case ola::rdm::RDM_DUB_RESPONSE: - status_code = ola::e133::SC_E133_RDM_INVALID_RESPONSE; - break; - } + ola::e133::E133StatusCode status_code = RDMResponseCodeToE133Status( + response_code); SendStatusMessage(target, sequence_number, endpoint_id, status_code, description); return; diff --git a/tools/e133/EndpointManager.h b/tools/e133/EndpointManager.h index 4861aa4baa..86a5c49aa2 100644 --- a/tools/e133/EndpointManager.h +++ b/tools/e133/EndpointManager.h @@ -43,7 +43,10 @@ class EndpointManager { typedef ola::Callback1 EndpointNotificationCallback; typedef enum { ADD, REMOVE, BOTH } EndpointNoticationEvent; - EndpointManager() {} + EndpointManager() + : m_list_change_number(0) { + } + ~EndpointManager() {} uint32_t list_change_number() const { return m_list_change_number; } diff --git a/tools/rdmpro/rdm-sniffer.cpp b/tools/rdmpro/rdm-sniffer.cpp index dfe6d014bd..fce327a17c 100644 --- a/tools/rdmpro/rdm-sniffer.cpp +++ b/tools/rdmpro/rdm-sniffer.cpp @@ -232,6 +232,7 @@ void RDMSniffer::ProcessTuple(uint8_t control_byte, uint8_t data_byte) { // this is an actual byte of data switch (m_state) { case IDLE: + // fall through case MAB: m_state = DATA; m_frame.Reset(); From f758a17b6def35331370e18ccc53457f2cf7a438 Mon Sep 17 00:00:00 2001 From: Simon Newton Date: Fri, 2 Jan 2015 19:33:22 -0800 Subject: [PATCH 3/6] Fix the build. --- common/network/PosixInterfacePicker.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/network/PosixInterfacePicker.cpp b/common/network/PosixInterfacePicker.cpp index 801710267e..b74dda3810 100644 --- a/common/network/PosixInterfacePicker.cpp +++ b/common/network/PosixInterfacePicker.cpp @@ -72,9 +72,12 @@ using std::vector; vector PosixInterfacePicker::GetInterfaces( bool include_loopback) const { vector interfaces; + +#ifdef HAVE_SOCKADDR_DL_STRUCT string last_dl_iface_name; uint8_t hwlen = 0; char *hwaddr = NULL; +#endif // create socket to get iface config int sd = socket(PF_INET, SOCK_DGRAM, 0); From f4d7b8bbb4a04dd996231dea3cec1730511c130b Mon Sep 17 00:00:00 2001 From: Simon Newton Date: Fri, 2 Jan 2015 20:55:59 -0800 Subject: [PATCH 4/6] Fix the build. --- tools/e133/E133Device.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/e133/E133Device.cpp b/tools/e133/E133Device.cpp index e3f85fa5b5..f8de34d03e 100644 --- a/tools/e133/E133Device.cpp +++ b/tools/e133/E133Device.cpp @@ -86,6 +86,7 @@ ola::e133::E133StatusCode RDMResponseCodeToE133Status( case ola::rdm::RDM_DUB_RESPONSE: return ola::e133::SC_E133_RDM_INVALID_RESPONSE; } + return ola::e133::SC_E133_RDM_INVALID_RESPONSE; } E133Device::E133Device(ola::io::SelectServerInterface *ss, From c1571395ce3e01c086df55ea983793ef79670a00 Mon Sep 17 00:00:00 2001 From: Simon Newton Date: Fri, 2 Jan 2015 21:59:39 -0800 Subject: [PATCH 5/6] Fix some more defects. --- common/system/Limits.cpp | 4 +- common/web/JsonLexer.cpp | 1 + include/ola/strings/FormatPrivate.h | 3 ++ olad/RDMHTTPModule.cpp | 3 +- plugins/artnet/ArtNetNode.cpp | 2 + plugins/artnet/ArtNetNode.h | 9 +++-- plugins/dummy/DummyDevice.h | 23 ++++++------ plugins/e131/e131/TransportHeader.h | 45 +++++++++++----------- plugins/karate/KarateLight.cpp | 58 +++++++++++++++-------------- tools/e133/E133Device.cpp | 1 + 10 files changed, 81 insertions(+), 68 deletions(-) diff --git a/common/system/Limits.cpp b/common/system/Limits.cpp index 6097eaad21..37ae9c7504 100644 --- a/common/system/Limits.cpp +++ b/common/system/Limits.cpp @@ -33,7 +33,7 @@ namespace system { bool GetRLimit(int resource, struct rlimit *lim) { int r = getrlimit(resource, lim); if (r) { - OLA_WARN << "getrlimit(" << resource << "): " << strerror(r); + OLA_WARN << "getrlimit(" << resource << "): " << strerror(errno); return false; } return true; @@ -42,7 +42,7 @@ bool GetRLimit(int resource, struct rlimit *lim) { bool SetRLimit(int resource, const struct rlimit &lim) { int r = setrlimit(resource, &lim); if (r) { - OLA_WARN << "setrlimit(" << resource << "): " << strerror(r); + OLA_WARN << "setrlimit(" << resource << "): " << strerror(errno); return false; } return true; diff --git a/common/web/JsonLexer.cpp b/common/web/JsonLexer.cpp index 07dcaea0c0..af50714180 100644 --- a/common/web/JsonLexer.cpp +++ b/common/web/JsonLexer.cpp @@ -196,6 +196,7 @@ static bool ParseNumber(const char **input, JsonParserInterface *parser) { switch (**input) { case '-': negative_exponent = true; + // fall through case '+': (*input)++; break; diff --git a/include/ola/strings/FormatPrivate.h b/include/ola/strings/FormatPrivate.h index a2a3f13b1e..72c9488ee1 100644 --- a/include/ola/strings/FormatPrivate.h +++ b/include/ola/strings/FormatPrivate.h @@ -57,8 +57,11 @@ struct _ToHex { }; inline uint32_t _HexCast(uint8_t v) { return v; } +inline uint32_t _HexCast(int8_t v) { return static_cast(v); } inline uint16_t _HexCast(uint16_t v) { return v; } +inline uint16_t _HexCast(int16_t v) { return static_cast(v); } inline uint32_t _HexCast(uint32_t v) { return v; } +inline uint32_t _HexCast(int32_t v) { return static_cast(v); } } // namespace strings } // namespace ola diff --git a/olad/RDMHTTPModule.cpp b/olad/RDMHTTPModule.cpp index 36b4c29ed3..4bfd8a3825 100644 --- a/olad/RDMHTTPModule.cpp +++ b/olad/RDMHTTPModule.cpp @@ -157,7 +157,8 @@ RDMHTTPModule::RDMHTTPModule(HTTPServer *http_server, : m_server(http_server), m_client(client), m_shim(client), - m_rdm_api(&m_shim) { + m_rdm_api(&m_shim), + m_pid_store(NULL) { m_server->RegisterHandler( "/rdm/run_discovery", diff --git a/plugins/artnet/ArtNetNode.cpp b/plugins/artnet/ArtNetNode.cpp index 91ed51ad5a..116332baa3 100644 --- a/plugins/artnet/ArtNetNode.cpp +++ b/plugins/artnet/ArtNetNode.cpp @@ -199,6 +199,8 @@ ArtNetNodeImpl::ArtNetNodeImpl(const ola::network::Interface &iface, m_always_broadcast(options.always_broadcast), m_use_limited_broadcast_address(options.use_limited_broadcast_address), m_in_configuration_mode(false), + m_artpoll_required(false), + m_artpollreply_required(false), m_interface(iface), m_socket(socket) { diff --git a/plugins/artnet/ArtNetNode.h b/plugins/artnet/ArtNetNode.h index c292252676..d783d112a3 100644 --- a/plugins/artnet/ArtNetNode.h +++ b/plugins/artnet/ArtNetNode.h @@ -407,9 +407,6 @@ class ArtNetNodeImpl { ola::network::Interface m_interface; std::auto_ptr m_socket; - ArtNetNodeImpl(const ArtNetNodeImpl&); - ArtNetNodeImpl& operator=(const ArtNetNodeImpl&); - /** * @brief Called when there is data on this socket */ @@ -655,6 +652,8 @@ class ArtNetNodeImpl { static const unsigned int RDM_REQUEST_QUEUE_LIMIT = 100; // How long to wait for a response to an RDM Request static const unsigned int RDM_REQUEST_TIMEOUT_MS = 2000; + + DISALLOW_COPY_AND_ASSIGN(ArtNetNodeImpl); }; @@ -687,6 +686,8 @@ class ArtNetNodeImplRDMWrapper private: ArtNetNodeImpl *m_impl; uint8_t m_port_id; + + DISALLOW_COPY_AND_ASSIGN(ArtNetNodeImplRDMWrapper); }; @@ -853,6 +854,8 @@ class ArtNetNode { * @return true if the port id is valid, false otherwise */ bool CheckInputPortId(uint8_t port_id); + + DISALLOW_COPY_AND_ASSIGN(ArtNetNode); }; } // namespace artnet } // namespace plugin diff --git a/plugins/dummy/DummyDevice.h b/plugins/dummy/DummyDevice.h index de19fa0a18..bdea24caa9 100644 --- a/plugins/dummy/DummyDevice.h +++ b/plugins/dummy/DummyDevice.h @@ -34,21 +34,20 @@ namespace dummy { class DummyDevice: public Device { public: - DummyDevice( - AbstractPlugin *owner, - const std::string &name, - const DummyPort::Options &port_options) - : Device(owner, name), - m_port_options(port_options) { - } - std::string DeviceId() const { return "1"; } + DummyDevice( + AbstractPlugin *owner, + const std::string &name, + const DummyPort::Options &port_options) + : Device(owner, name), + m_port_options(port_options) { + } + + std::string DeviceId() const { return "1"; } protected: - uint16_t m_number_of_devices; - uint16_t m_number_of_subdevices; - const DummyPort::Options m_port_options; + const DummyPort::Options m_port_options; - bool StartHook(); + bool StartHook(); }; } // namespace dummy } // namespace plugin diff --git a/plugins/e131/e131/TransportHeader.h b/plugins/e131/e131/TransportHeader.h index 237937fdc3..d7e7622df2 100644 --- a/plugins/e131/e131/TransportHeader.h +++ b/plugins/e131/e131/TransportHeader.h @@ -35,34 +35,35 @@ namespace e131 { */ class TransportHeader { public: - enum TransportType { - TCP, - UDP, - }; + enum TransportType { + TCP, + UDP, + UNDEFINED, + }; - TransportHeader() {} - TransportHeader(const ola::network::IPV4SocketAddress &source, - TransportType type) - : m_source(source), - m_transport_type(type) {} + TransportHeader() : m_transport_type(UNDEFINED) {} + TransportHeader(const ola::network::IPV4SocketAddress &source, + TransportType type) + : m_source(source), + m_transport_type(type) {} - ~TransportHeader() {} - const ola::network::IPV4SocketAddress& Source() const { return m_source; } - TransportType Transport() const { return m_transport_type; } + ~TransportHeader() {} + const ola::network::IPV4SocketAddress& Source() const { return m_source; } + TransportType Transport() const { return m_transport_type; } - bool operator==(const TransportHeader &other) const { - return (m_source == other.m_source && - m_transport_type == other.m_transport_type); - } + bool operator==(const TransportHeader &other) const { + return (m_source == other.m_source && + m_transport_type == other.m_transport_type); + } - void operator=(const TransportHeader &other) { - m_source = other.m_source; - m_transport_type = other.m_transport_type; - } + void operator=(const TransportHeader &other) { + m_source = other.m_source; + m_transport_type = other.m_transport_type; + } private: - ola::network::IPV4SocketAddress m_source; - TransportType m_transport_type; + ola::network::IPV4SocketAddress m_source; + TransportType m_transport_type; }; } // namespace e131 } // namespace plugin diff --git a/plugins/karate/KarateLight.cpp b/plugins/karate/KarateLight.cpp index 882e819e08..529ed264c4 100644 --- a/plugins/karate/KarateLight.cpp +++ b/plugins/karate/KarateLight.cpp @@ -34,6 +34,7 @@ #include "ola/DmxBuffer.h" #include "ola/Logging.h" #include "ola/io/IOUtils.h" +#include "ola/strings/Format.h" #include "plugins/karate/KarateLight.h" namespace ola { @@ -48,6 +49,7 @@ using std::string; */ KarateLight::KarateLight(const string &dev) : m_devname(dev), + m_fd(-1), m_fw_version(0), m_hw_version(0), m_nChannels(0), @@ -67,9 +69,11 @@ KarateLight::~KarateLight() { void KarateLight::Close() { // remove lock and close file - flock(m_fd, LOCK_UN); - tcflush(m_fd, TCIOFLUSH); - close(m_fd); + if (m_fd >= 0) { + flock(m_fd, LOCK_UN); + tcflush(m_fd, TCIOFLUSH); + close(m_fd); + } m_active = false; } @@ -123,9 +127,9 @@ bool KarateLight::Init() { } // clear possible junk data still in the systems fifo - int bytesread = 1; - while (bytesread > 0) { - bytesread = read(m_fd, rd_buffer, CMD_MAX_LENGTH); + int bytes_read = 1; + while (bytes_read > 0) { + bytes_read = read(m_fd, rd_buffer, CMD_MAX_LENGTH); } // read firmware version @@ -187,10 +191,10 @@ bool KarateLight::Init() { OLA_INFO << "successfully initalized device " << m_devname << " with firmware version 0x" - << std::hex << static_cast(m_fw_version) + << strings::ToHex(m_fw_version) << ", hardware-revision = 0x" - << std::hex << static_cast(m_hw_version) - << ", channel_count = " << std::dec << m_nChannels + << strings::ToHex(m_hw_version) + << ", channel_count = " << m_nChannels << ", dmx_offset = " << m_dmx_offset; // set channels to black @@ -228,17 +232,16 @@ bool KarateLight::SetColors(const DmxBuffer &da) { * @brief Tries to read an answer from the device * @param rd_data buffer for the received data (excluding the header) * @param rd_len number of bytes to read (excluding the header), will be - * overwritten with the number of bytes received in the case - * of a mismatch + * overwritten with the number of bytes received in the case + * of a mismatch * @return true on success */ bool KarateLight::ReadBack(uint8_t *rd_data, uint8_t *rd_len) { - int bytesread = 0; uint8_t rd_buffer[CMD_MAX_LENGTH]; // read header (4 bytes) - bytesread = read(m_fd, rd_buffer, CMD_DATA_START); - if (bytesread != CMD_DATA_START) { + int bytes_read = read(m_fd, rd_buffer, CMD_DATA_START); + if (bytes_read != CMD_DATA_START) { if (errno != EINTR) { // this is also true for EAGAIN OLA_WARN << "Could not read 4 bytes (header) from " << m_devname << " ErrorCode: " << strerror(errno); @@ -246,15 +249,16 @@ bool KarateLight::ReadBack(uint8_t *rd_data, uint8_t *rd_len) { return false; } } - bytesread = 0; + bytes_read = 0; // read payload-data (if there is any) - if (rd_buffer[CMD_HD_LEN] > 0) { - // we wont enter this loop if there are no bytes to receive - bytesread = read(m_fd, &rd_buffer[CMD_DATA_START], rd_buffer[CMD_HD_LEN]); - if (bytesread != rd_buffer[CMD_HD_LEN]) { + uint8_t payload_size = rd_buffer[CMD_HD_LEN]; + if (payload_size > 0u) { + // we won't enter this loop if there are no bytes to receive + bytes_read = read(m_fd, &rd_buffer[CMD_DATA_START], payload_size); + if (bytes_read != payload_size) { if (errno != EINTR) { // this is also true for EAGAIN (timeout) - OLA_WARN << "Reading > " << static_cast(rd_buffer[CMD_HD_LEN]) + OLA_WARN << "Reading > " << static_cast(payload_size) << " < bytes payload from " << m_devname << " ErrorCode: " << strerror(errno); KarateLight::Close(); @@ -264,11 +268,10 @@ bool KarateLight::ReadBack(uint8_t *rd_data, uint8_t *rd_len) { } // verify data-length - if ((*rd_len != rd_buffer[CMD_HD_LEN]) || - (bytesread != rd_buffer[CMD_HD_LEN])) { - OLA_WARN << "Number of bytes read > " << bytesread + if (*rd_len != payload_size) { + OLA_WARN << "Number of bytes read > " << bytes_read << " < does not match number of bytes expected > " - << static_cast(rd_buffer[CMD_HD_LEN]) + << static_cast(payload_size) << " <"; KarateLight::Close(); return false; @@ -276,15 +279,14 @@ bool KarateLight::ReadBack(uint8_t *rd_data, uint8_t *rd_len) { // verify checksum int checksum = 0; - for (int i = 0; i < (bytesread + CMD_DATA_START); i++) { + for (int i = 0; i < bytes_read + CMD_DATA_START; i++) { if (i != CMD_HD_CHECK) { checksum ^= rd_buffer[i]; } } if (checksum != rd_buffer[CMD_HD_CHECK]) { OLA_WARN << "Checkum verification of incoming data failed. " - << "Data-checkum is: 0x" << std::hex - << static_cast(checksum) + << "Data-checkum is: " << strings::ToHex(checksum) << " but the device said it would be 0x" << static_cast(rd_buffer[CMD_HD_CHECK]); KarateLight::Close(); @@ -293,7 +295,7 @@ bool KarateLight::ReadBack(uint8_t *rd_data, uint8_t *rd_len) { } // prepare data - *rd_len = static_cast(bytesread); + *rd_len = static_cast(bytes_read); memcpy(rd_data, &rd_buffer[CMD_DATA_START], *rd_len); return true; diff --git a/tools/e133/E133Device.cpp b/tools/e133/E133Device.cpp index f8de34d03e..d98315207a 100644 --- a/tools/e133/E133Device.cpp +++ b/tools/e133/E133Device.cpp @@ -98,6 +98,7 @@ E133Device::E133Device(ola::io::SelectServerInterface *ss, m_message_builder(cid, "OLA Device"), m_endpoint_manager(endpoint_manager), m_root_endpoint(NULL), + m_root_rdm_device(NULL), m_incoming_udp_transport(&m_udp_socket, &m_root_inflator) { m_root_inflator.AddInflator(&m_e133_inflator); m_e133_inflator.AddInflator(&m_rdm_inflator); From 6101023e24763c7ffc57ba6ccb22d7d1f1996b0e Mon Sep 17 00:00:00 2001 From: Simon Newton Date: Fri, 2 Jan 2015 22:04:21 -0800 Subject: [PATCH 6/6] Fix the build. --- common/system/Limits.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/common/system/Limits.cpp b/common/system/Limits.cpp index 37ae9c7504..ef2c6c3673 100644 --- a/common/system/Limits.cpp +++ b/common/system/Limits.cpp @@ -22,6 +22,7 @@ #ifndef _WIN32 #include "ola/system/Limits.h" +#include #include #include