MMDevice: Remove need for _CRT_SECURE_NO_WARNINGS

That is, remove use of strcpy(), strncpy(), and getenv().

Replace str[n]cpy() with snprintf(). Add tests in the case of
CDeviceUtils::CopyLimitedString() (previous implementation had a bug
where it didn't null-terminate if the source string was exactly
MM::MaxStrLength - 1 chars long).

The use of getenv() was dead code, so remove entirely.

MMDevice/DeviceUtils.cpp

@@ -34,18 +34,16 @@
 char CDeviceUtils::m_pszBuffer[MM::MaxStrLength]={""};
 
 /**
- * Copies strings with predefined size limit.
+ * Copies string up to MM::MaxStrLength - 1 characters, truncating if necessary
+ * and ensuring the result is null-terminated.
+ * 
+ * Behavior is undefined unless dest points to a buffer with size at least
+ * MM::MaxStrLength and src points to a null-terminated string.
  */
-bool CDeviceUtils::CopyLimitedString(char* target, const char* source)
+bool CDeviceUtils::CopyLimitedString(char* dest, const char* src)
 {
-   strncpy(target, source, MM::MaxStrLength - 1);
-   if ((MM::MaxStrLength - 1) < strlen(source))
-   {
-      target[MM::MaxStrLength - 1] = 0;
-      return false; // string truncated
-   }
-   else
-      return true;
+   snprintf(dest, MM::MaxStrLength, "%s", src);
+   return std::strlen(src) <= MM::MaxStrLength;
 }
 
 /**
@@ -169,22 +167,3 @@ void CDeviceUtils::NapMicros(unsigned long period)
    usleep(period);
 #endif
 }
-
-
-bool CDeviceUtils::CheckEnvironment(std::string env)
-{
-   bool bvalue = false;
-   if( 0 < env.length())
-   {
-      char *pvalue = ::getenv(env.c_str());
-      if( 0 != pvalue)
-      {
-         if( 0 != *pvalue)
-         {
-            char initial =  (char)tolower(*pvalue);
-            bvalue = ('0' != initial) && ('f' != initial) && ( 'n' != initial);
-         }
-      }
-   }
-   return bvalue;
-}

MMDevice/DeviceUtils.h

@@ -37,7 +37,6 @@ class CDeviceUtils
    static void SleepMs(long ms);
    static void NapMicros(unsigned long microsecs);
    static std::string HexRep(std::vector<unsigned char>  );
-   static bool CheckEnvironment(std::string environment);
 private:
    static char m_pszBuffer[MM::MaxStrLength];
 };

MMDevice/MMDeviceConstants.h

@@ -78,6 +78,9 @@
 
 
 namespace MM {
+   // Maximum length copied into various char buffers.
+   // Code providing buffer should assume excludes null terminator.
+   // Code filling buffer should assume includes null terminator.
    const int MaxStrLength = 1024;
 
    // system-wide property names

MMDevice/ModuleInterface.cpp

@@ -85,7 +85,7 @@ MODULE_API bool GetDeviceName(unsigned deviceIndex, char* name, unsigned bufLen)
    if (deviceName.size() >= bufLen)
       return false; // buffer too small, can't truncate the name
 
-   strcpy(name, deviceName.c_str());
+   std::snprintf(name, bufLen, "%s", deviceName.c_str());
    return true;
 }
 
@@ -115,8 +115,7 @@ MODULE_API bool GetDeviceDescription(const char* deviceName, char* description,
    if (it == g_registeredDevices.end())
       return false;
 
-   strncpy(description, it->description_.c_str(), bufLen - 1);
-
+   std::snprintf(description, bufLen, "%s", it->description_.c_str());
    return true;
 }
 

MMDevice/meson.build

@@ -48,9 +48,7 @@ mmdevice_lib = static_library(
     'MMDevice',
     sources: mmdevice_sources,
     include_directories: mmdevice_include_dir,
-    cpp_args: build_mode_args + [
-        '-D_CRT_SECURE_NO_WARNINGS', # TODO Eliminate the need
-    ],
+    cpp_args: build_mode_args,
     # MMDevice does not depend on any external library. This is a big advantage
     # in simplifing its usage (given hundreds of device adapters depending on
     # MMDevice), so think twice before adding dependencies.

MMDevice/unittest/DeviceUtils-Tests.cpp

@@ -0,0 +1,26 @@
+#include <catch2/catch_all.hpp>
+
+#include "DeviceUtils.h"
+
+#include <string>
+#include <vector>
+
+TEST_CASE("CopyLimitedString truncates", "[CopyLimitedString]")
+{
+   std::vector<char> dest(MM::MaxStrLength, '.');
+   const std::string src(MM::MaxStrLength + 10, '*');
+   CHECK_FALSE(CDeviceUtils::CopyLimitedString(dest.data(), src.c_str()));
+   CHECK(dest[0] == '*');
+   CHECK(dest[MM::MaxStrLength - 2] == '*');
+   CHECK(dest[MM::MaxStrLength - 1] == '\0');
+}
+
+TEST_CASE("CopyLimitedString max untruncated len", "[CopyLimitedString]")
+{
+   std::vector<char> dest(MM::MaxStrLength, '.');
+   const std::string src(MM::MaxStrLength - 1, '*');
+   CHECK(CDeviceUtils::CopyLimitedString(dest.data(), src.c_str()));
+   CHECK(dest[0] == '*');
+   CHECK(dest[MM::MaxStrLength - 2] == '*');
+   CHECK(dest[MM::MaxStrLength - 1] == '\0');
+}

MMDevice/unittest/meson.build

@@ -10,6 +10,7 @@ catch2_with_main_dep = dependency(
 )
 
 mmdevice_test_sources = files(
+    'DeviceUtils-Tests.cpp',
     'FloatPropertyTruncation-Tests.cpp',
     'MMTime-Tests.cpp',
 )