Merge pull request #586 from nomis52/master

Refactor string functions, fix many defects.

NEWS

@@ -1,4 +1,4 @@
-x/y/2014 ola-0.9.4
+x/y/2015 ola-0.9.4
  Features:
  * Added support for hotplugging USB devices using the usbdmx plugin, #374.
  * Added support for setting the thread scheduling options, #399.

common/Makefile.mk

@@ -24,6 +24,7 @@ include common/network/Makefile.mk
 include common/protocol/Makefile.mk
 include common/rdm/Makefile.mk
 include common/rpc/Makefile.mk
+include common/strings/Makefile.mk
 include common/system/Makefile.mk
 include common/testing/Makefile.mk
 include common/thread/Makefile.mk

common/file/Util.cpp

@@ -109,13 +109,14 @@ bool FindMatchingFiles(const string &directory,
   DIR *dp;
   struct dirent dir_ent;
   struct dirent *dir_ent_p;
-  if ((dp  = opendir(directory.data())) == NULL) {
+  if ((dp = opendir(directory.data())) == NULL) {
     OLA_WARN << "Could not open " << directory << ":" << strerror(errno);
     return false;
   }
 
   if (readdir_r(dp, &dir_ent, &dir_ent_p)) {
     OLA_WARN << "readdir_r(" << directory << "): " << strerror(errno);
+    closedir(dp);
     return false;
   }
 
@@ -130,6 +131,7 @@ bool FindMatchingFiles(const string &directory,
     }
     if (readdir_r(dp, &dir_ent, &dir_ent_p)) {
       OLA_WARN << "readdir_r(" << directory << "): " << strerror(errno);
+      closedir(dp);
       return false;
     }
   }

common/io/IOStack.cpp

@@ -244,9 +244,10 @@ void IOStack::PrependBlock() {
   MemoryBlock *block = m_pool->Allocate();
   if (!block) {
     OLA_FATAL << "Failed to allocate block, we're out of memory!";
+  } else {
+    block->SeekBack();  // put the block into prepend mode
+    m_blocks.push_front(block);
   }
-  block->SeekBack();  // put the block into prepend mode
-  m_blocks.push_front(block);
 }
 }  // namespace io
 }  // namespace ola

common/rdm/DiscoveryAgent.cpp

@@ -60,7 +60,8 @@ DiscoveryAgent::DiscoveryAgent(DiscoveryTargetInterface *target)
       m_branch_callback(
         ola::NewCallback(this, &DiscoveryAgent::BranchComplete)),
       m_muting_uid(0, 0),
-      m_mute_attempts(0) {
+      m_mute_attempts(0),
+      m_tree_corrupt(false) {
 }
 
 

common/rdm/RDMHelper.cpp

@@ -994,15 +994,15 @@ string StatusMessageIdToString(uint16_t message_id,
       // Data Value shall be a signed integer." but I'm sure it's what was
       // intended. The same thing is technically true with the slots too.
       str << "Proxy Drop: PID "
-          << ToHex(reinterpret_cast<uint16_t&>(data1)) << " at TN "
+          << strings::ToHex(reinterpret_cast<uint16_t&>(data1)) << " at TN "
           << data2;
       break;
     case STS_ASC_RXOK:
-      str << "DMX ASC " << ToHex(reinterpret_cast<uint16_t&>(data1))
+      str << "DMX ASC " << strings::ToHex(reinterpret_cast<uint16_t&>(data1))
           << " received OK";
       break;
     case STS_ASC_DROPPED:
-      str << "DMX ASC " << ToHex(reinterpret_cast<uint16_t&>(data1))
+      str << "DMX ASC " << strings::ToHex(reinterpret_cast<uint16_t&>(data1))
           << " now dropped";
       break;
     case STS_DMXNSCNONE:

common/rdm/ResponderHelper.cpp

@@ -23,18 +23,20 @@
 #include <algorithm>
 #include <string>
 #include <vector>
-#include "ola/base/Macro.h"
 #include "ola/Clock.h"
 #include "ola/Constants.h"
 #include "ola/Logging.h"
+#include "ola/base/Array.h"
+#include "ola/base/Macro.h"
+#include "ola/network/IPV4Address.h"
 #include "ola/network/Interface.h"
 #include "ola/network/InterfacePicker.h"
-#include "ola/network/IPV4Address.h"
 #include "ola/network/MACAddress.h"
 #include "ola/network/NetworkUtils.h"
+#include "ola/rdm/RDMEnums.h"
 #include "ola/rdm/ResponderHelper.h"
 #include "ola/rdm/ResponderSensor.h"
-#include "ola/rdm/RDMEnums.h"
+#include "ola/strings/Utils.h"
 
 namespace ola {
 namespace rdm {
@@ -484,9 +486,9 @@ const RDMResponse *ResponderHelper::GetSensorDefinition(
   sensor_definition.normal_min = HostToNetwork(sensor->NormalMin());
   sensor_definition.normal_max = HostToNetwork(sensor->NormalMax());
   sensor_definition.recorded_support = sensor->RecordedSupportBitMask();
-  strncpy(sensor_definition.description, sensor->Description().c_str(),
-          sizeof(sensor_definition.description));
-
+  strings::CopyToFixedLengthBuffer(sensor->Description(),
+                                   sensor_definition.description,
+                                   arraysize(sensor_definition.description));
   return GetResponseFromData(
     request,
     reinterpret_cast<const uint8_t*>(&sensor_definition),

common/rdm/ResponderSettings.cpp

@@ -17,13 +17,16 @@
  * Copyright (C) 2013 Simon Newton
  */
 
+#include "ola/rdm/ResponderSettings.h"
 
-#include <ola/network/NetworkUtils.h>
-#include <ola/rdm/RDMCommand.h>
-#include <ola/rdm/ResponderSettings.h>
 #include <algorithm>
 #include <string>
 
+#include "ola/base/Array.h"
+#include "ola/network/NetworkUtils.h"
+#include "ola/rdm/RDMCommand.h"
+#include "ola/strings/Utils.h"
+
 namespace ola {
 namespace rdm {
 
@@ -37,10 +40,11 @@ unsigned int BasicSetting::GenerateDescriptionResponse(uint8_t index,
                                                        uint8_t *data) const {
   description_s *output = reinterpret_cast<description_s*>(data);
   output->setting = index;
-  strncpy(output->description, m_description.c_str(), MAX_RDM_STRING_LENGTH);
+  strings::CopyToFixedLengthBuffer(m_description, output->description,
+                                   arraysize(output->description));
   return (sizeof(description_s) - MAX_RDM_STRING_LENGTH +
           std::min(static_cast<size_t>(MAX_RDM_STRING_LENGTH),
-                   strlen(output->description)));
+                   m_description.size()));
 }
 
 FrequencyModulationSetting::FrequencyModulationSetting(const ArgType &arg)
@@ -54,9 +58,9 @@ unsigned int FrequencyModulationSetting::GenerateDescriptionResponse(
   description_s *output = reinterpret_cast<description_s*>(data);
   output->setting = index;
   output->frequency = ola::network::HostToNetwork(m_frequency);
-  strncpy(output->description, m_description.c_str(), MAX_RDM_STRING_LENGTH);
-  return (sizeof(description_s) - MAX_RDM_STRING_LENGTH +
-          strlen(output->description));
+  strings::CopyToFixedLengthBuffer(m_description, output->description,
+                                   arraysize(output->description));
+  return (sizeof(description_s) - MAX_RDM_STRING_LENGTH + m_description.size());
 }
 }  // namespace rdm
 }  // namespace ola

common/strings/Format.cpp

@@ -0,0 +1,75 @@
+/*
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ * Format.cpp
+ * Formatting functions for basic types.
+ * Copyright (C) 2005 Simon Newton
+ */
+
+#include "ola/strings/Format.h"
+
+#include <iomanip>
+#include <sstream>
+#include <string>
+
+namespace ola {
+namespace strings {
+
+using std::endl;
+using std::ostringstream;
+using std::string;
+
+string IntToString(int i) {
+  ostringstream str;
+  str << i;
+  return str.str();
+}
+
+string IntToString(unsigned int i) {
+  ostringstream str;
+  str << i;
+  return str.str();
+}
+
+void FormatData(std::ostream *out,
+                const uint8_t *data,
+                unsigned int length,
+                unsigned int indent,
+                unsigned int byte_per_line) {
+  ostringstream raw, ascii;
+  raw << std::hex;
+  for (unsigned int i = 0; i != length; i++) {
+    raw << std::setfill('0') << std::setw(2)
+        << static_cast<unsigned int>(data[i]) << " ";
+    if (isprint(data[i])) {
+      ascii << data[i];
+    } else {
+      ascii << ".";
+    }
+
+    if (i % byte_per_line == byte_per_line - 1) {
+      *out << string(indent, ' ') << raw.str() << " " << ascii.str() << endl;
+      raw.str("");
+      ascii.str("");
+    }
+  }
+  if (length % byte_per_line != 0) {
+    // pad if needed
+    raw << string(3 * (byte_per_line - (length % byte_per_line)), ' ');
+    *out << string(indent, ' ') << raw.str() << " " << ascii.str() << endl;
+  }
+}
+}  // namespace strings
+}  // namespace ola

common/strings/Makefile.mk

@@ -0,0 +1,14 @@
+# LIBRARIES
+##################################################
+common_libolacommon_la_SOURCES += \
+    common/strings/Format.cpp \
+    common/strings/Utils.cpp
+
+# TESTS
+################################################
+test_programs += common/strings/UtilsTester
+
+common_strings_UtilsTester_SOURCES = \
+    common/strings/UtilsTest.cpp
+common_strings_UtilsTester_CXXFLAGS = $(COMMON_TESTING_FLAGS)
+common_strings_UtilsTester_LDADD = $(COMMON_TESTING_LIBS)

common/strings/Utils.cpp

@@ -0,0 +1,39 @@
+/*
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ * Utils.cpp
+ * Miscellaneous string functions.
+ * Copyright (C) 2014 Simon Newton
+ */
+
+#include "ola/strings/Utils.h"
+
+#include <string.h>
+#include <string>
+
+namespace ola {
+namespace strings {
+
+using std::string;
+
+void CopyToFixedLengthBuffer(const std::string &input,
+                             char *buffer,
+                             unsigned int size) {
+  // buffer may not be NULL terminated.
+  // coverity(BUFFER_SIZE_WARNING)
+  strncpy(buffer, input.c_str(), size);
+}
+}  // namespace strings
+}  // namespace ola

common/strings/UtilsTest.cpp

@@ -0,0 +1,66 @@
+/*
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ * UtilsTest.cpp
+ * Unittest for strings Util functions.
+ * Copyright (C) 2014 Simon Newton
+ */
+
+#include <cppunit/extensions/HelperMacros.h>
+#include <string>
+
+#include "ola/base/Array.h"
+#include "ola/strings/Utils.h"
+#include "ola/testing/TestUtils.h"
+
+using ola::strings::CopyToFixedLengthBuffer;
+using std::string;
+
+class UtilsTest: public CppUnit::TestFixture {
+  CPPUNIT_TEST_SUITE(UtilsTest);
+  CPPUNIT_TEST(testCopyToFixedLengthBuffer);
+  CPPUNIT_TEST_SUITE_END();
+
+ public:
+    void testCopyToFixedLengthBuffer();
+};
+
+
+CPPUNIT_TEST_SUITE_REGISTRATION(UtilsTest);
+
+/*
+ * Test the CopyToFixedLengthBuffer function
+ */
+void UtilsTest::testCopyToFixedLengthBuffer() {
+  char buffer[6];
+  const string short_input("foo");
+  const string input("foobar");
+  const string oversized_input("foobarbaz");
+
+  const char short_output[] = {'f', 'o', 'o', 0, 0, 0};
+  CopyToFixedLengthBuffer(short_input, buffer, arraysize(buffer));
+  OLA_ASSERT_DATA_EQUALS(short_output, arraysize(short_output),
+                         buffer, arraysize(buffer));
+
+  const char output[] = {'f', 'o', 'o', 'b', 'a', 'r'};
+  CopyToFixedLengthBuffer(input, buffer, arraysize(buffer));
+  OLA_ASSERT_DATA_EQUALS(output, arraysize(output),
+                         buffer, arraysize(buffer));
+
+  const char oversized_output[] = {'f', 'o', 'o', 'b', 'a', 'r'};
+  CopyToFixedLengthBuffer(oversized_input, buffer, arraysize(buffer));
+  OLA_ASSERT_DATA_EQUALS(oversized_output, arraysize(oversized_output),
+                         buffer, arraysize(buffer));
+}

common/testing/TestUtils.cpp

@@ -71,5 +71,16 @@ void ASSERT_DATA_EQUALS(unsigned int line,
   }
   CPPUNIT_ASSERT_MESSAGE(message, data_matches);
 }
+
+void ASSERT_DATA_EQUALS(unsigned int line,
+                        const char *expected,
+                        unsigned int expected_length,
+                        const char *actual,
+                        unsigned int actual_length) {
+  ASSERT_DATA_EQUALS(line, reinterpret_cast<const uint8_t*>(expected),
+                     expected_length,
+                     reinterpret_cast<const uint8_t*>(actual),
+                     actual_length);
+}
 }  // namespace testing
 }  // namespace ola