Bubble up Warnings as Errors (#1041)

* Werror on linux and fix up warnings
* Enable and fix warnings with msvc

CMakeLists.txt

@@ -410,6 +410,24 @@ add_executable(ni_grpc_device_server
    ${calibrationoperations_restricted_grpc_srcs}
    ${nidriver_service_srcs})
 
+# Enable warnings only on source that we own, not generated code or dependencies
+file(GLOB_RECURSE SOURCE_TO_ENABLE_WARNINGS "source/*.cpp")
+if(WIN32)
+  set_source_files_properties(
+    ${SOURCE_TO_ENABLE_WARNINGS}
+    APPEND
+    PROPERTIES
+    COMPILE_OPTIONS "/WX"
+  )
+else()
+  set_source_files_properties(
+    ${SOURCE_TO_ENABLE_WARNINGS}
+    APPEND
+    PROPERTIES
+    COMPILE_OPTIONS "-Werror"
+  )
+endif()
+
 if(WIN32)
   set(version_generated_dir "${CMAKE_CURRENT_BINARY_DIR}/version")
   file(MAKE_DIRECTORY ${version_generated_dir})
@@ -557,10 +575,18 @@ target_link_libraries(IntegrationTestsRunner
     Threads::Threads
     nlohmann_json::nlohmann_json)
 
+# Ignore the use of deprecated functions in test code
+target_compile_definitions(IntegrationTestsRunner
+    PRIVATE _CRT_SECURE_NO_WARNINGS _WINSOCK_DEPRECATED_NO_WARNINGS)
+
 add_library(TestApi SHARED
     "source/tests/utilities/test_api.cpp")
 add_compile_definitions(TestApi TEST_API_BUILDING)
 
+# Ignore the use of deprecated functions in test code
+target_compile_definitions(TestApi
+    PRIVATE _CRT_SECURE_NO_WARNINGS)
+
 add_executable(UnitTestsRunner
     "imports/include/nierr_Status.cpp"
     "source/tests/utilities/run_all_tests.cpp"
@@ -635,6 +661,10 @@ if(MSVC)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /bigobj /D_SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING")
 endif(MSVC)
 
+# Ignore the use of deprecated functions in test code
+target_compile_definitions(UnitTestsRunner
+    PRIVATE _CRT_SECURE_NO_WARNINGS _WINSOCK_DEPRECATED_NO_WARNINGS)
+
 target_include_directories(UnitTestsRunner
     PRIVATE "${service_output_dir}"
     PRIVATE "${service_output_dir}/nifake"
@@ -778,6 +808,10 @@ target_link_libraries(SystemTestsRunner
     nlohmann_json::nlohmann_json
 )
 
+# Ignore the use of deprecated functions in test code
+target_compile_definitions(SystemTestsRunner
+    PRIVATE _CRT_SECURE_NO_WARNINGS _WINSOCK_DEPRECATED_NO_WARNINGS)
+
 add_custom_command(
   TARGET SystemTestsRunner POST_BUILD
   COMMAND  ${CMAKE_COMMAND} -E copy_directory

source/custom/nifake_service.custom.cpp

@@ -1,4 +1,10 @@
+
+#pragma warning(push)
+#pragma warning(disable : 4616)
+#pragma warning(disable : 4146)
+#pragma warning(disable : 4244)
 #include <nifake/nifake_service.h>
+#pragma warning(pop)
 
 namespace nifake_grpc {
 

source/custom/nirfmxinstr_restricted_service.custom.cpp

@@ -113,17 +113,17 @@ ::grpc::Status NiRFmxInstrRestrictedService::GetInitiaitedSnapshotStrings(
       convert_to_grpc(signal_names, &signal_names_utf8);
       response->set_signal_names(signal_names_utf8);
       nidevice_grpc::converters::trim_trailing_nulls(*(response->mutable_signal_names()));
-      signal_names_actual_size = signal_names_utf8.length() + 1;
+      signal_names_actual_size = static_cast<int32>(signal_names_utf8.length() + 1);
       std::string result_names_utf8;
       convert_to_grpc(result_names, &result_names_utf8);
       response->set_result_names(result_names_utf8);
       nidevice_grpc::converters::trim_trailing_nulls(*(response->mutable_result_names()));
-      result_names_actual_size = result_names_utf8.length() + 1;
+      result_names_actual_size = static_cast<int32>(result_names_utf8.length() + 1);
       std::string snapshot_identifiers_utf8;
       convert_to_grpc(snapshot_identifiers, &snapshot_identifiers_utf8);
       response->set_snapshot_identifiers(snapshot_identifiers_utf8);
       nidevice_grpc::converters::trim_trailing_nulls(*(response->mutable_snapshot_identifiers()));
-      snapshot_identifiers_actual_size = snapshot_identifiers_utf8.length() + 1;
+      snapshot_identifiers_actual_size = static_cast<int32>(snapshot_identifiers_utf8.length() + 1);
       response->mutable_snapshot_timestamp_array()->Resize(snapshot_timestamp_array_actual_size, 0);
     }
     response->set_status(status);
@@ -209,12 +209,12 @@ ::grpc::Status NiRFmxInstrRestrictedService::GetLatestConfigurationSnapshot(
       convert_to_grpc(signal_name, &signal_name_utf8);
       response->set_signal_name(signal_name_utf8);
       nidevice_grpc::converters::trim_trailing_nulls(*(response->mutable_signal_name()));
-      signal_name_actual_size = signal_name_utf8.length() + 1;
+      signal_name_actual_size = static_cast<int32>(signal_name_utf8.length() + 1);
       std::string snapshot_identifier_utf8;
       convert_to_grpc(snapshot_identifier, &snapshot_identifier_utf8);
       response->set_snapshot_identifier(snapshot_identifier_utf8);
       nidevice_grpc::converters::trim_trailing_nulls(*(response->mutable_snapshot_identifier()));
-      snapshot_identifier_actual_size = snapshot_identifier_utf8.length() + 1;
+      snapshot_identifier_actual_size = static_cast<int32>(snapshot_identifier_utf8.length() + 1);
     }
     response->set_status(status);
     response->set_personality_id(personality_id);

source/custom/nirfmxinstr_service.custom.cpp

@@ -89,7 +89,7 @@ ::grpc::Status NiRFmxInstrService::BuildPortString(::grpc::ServerContext* contex
       if (status < 0) {
         return ConvertApiErrorStatusForNiRFmxInstrHandle(context, status, 0);
       }
-      int32 selector_string_out_length = status + std::to_string(channel_number).length();  // AB#1835966: RFmx Instr BuildPortString2 Bug.
+      int32 selector_string_out_length = static_cast<int32>(status + std::to_string(channel_number).length());  // AB#1835966: RFmx Instr BuildPortString2 Bug.
 
       std::string selector_string_out;
       if (selector_string_out_length > 0) {

source/custom/nixnet_service.custom.cpp

@@ -880,7 +880,7 @@ ::grpc::Status NiXnetService::SetProperty(::grpc::ServerContext* context, const
       }
       case string_: {
         auto property_value_mbcs = convert_from_grpc<std::string>(request->str());
-        u32 property_size = property_value_mbcs.size();
+        u32 property_size = static_cast<u32>(property_value_mbcs.size());
         status = library_->SetProperty(session, property_id, property_size, const_cast<char*>(property_value_mbcs.c_str()));
         break;
       }
@@ -978,7 +978,7 @@ ::grpc::Status NiXnetService::SetSubProperty(::grpc::ServerContext* context, con
       }
       case string_: {
         auto property_value_mbcs = convert_from_grpc<std::string>(request->str());
-        u32 property_size = property_value_mbcs.size();
+        u32 property_size = static_cast<u32>(property_value_mbcs.size());
         status = library_->SetSubProperty(session, active_index, property_id, property_size, const_cast<char*>(property_value_mbcs.c_str()));
         break;
       }
@@ -1053,7 +1053,7 @@ ::grpc::Status NiXnetService::DbSetProperty(::grpc::ServerContext* context, cons
       }
       case string_: {
         auto property_value_mbcs = convert_from_grpc<std::string>(request->str());
-        u32 property_size = property_value_mbcs.size();
+        u32 property_size = static_cast<u32>(property_value_mbcs.size());
         status = library_->DbSetProperty(dbobject, property_id, property_size, const_cast<char*>(property_value_mbcs.c_str()));
         break;
       }

source/server/linux/daemonize.cpp

@@ -59,9 +59,9 @@ int create_pidfile(const std::string& identity) {
     exit(EXIT_FAILURE);
   }
 
-  size = snprintf(NULL, 0, "%ld\n", getpid());
+  size = snprintf(NULL, 0, "%d\n", getpid());
   std::string pid_str(size, ' ');
-  if (snprintf(&pid_str[0], size + 1, "%ld\n", getpid()) > size) {
+  if (snprintf(&pid_str[0], size + 1, "%d\n", getpid()) > size) {
     logging::log(logging::Level_Error, "creating pid string failed");
     exit(EXIT_FAILURE);
   }
@@ -76,7 +76,7 @@ int create_pidfile(const std::string& identity) {
     close(fd);
     exit(EXIT_FAILURE);
   }
-  if (write(fd, pid_str.c_str(), size) != pid_str.length()) {
+  if (write(fd, pid_str.c_str(), size) != static_cast<ssize_t>(pid_str.length())) {
     logging::log(logging::Level_Error, "writing pid file failed: %d (%s)", errno, strerror(errno));
     close(fd);
     exit(EXIT_FAILURE);

source/tests/integration/ni_fake_service_tests_endtoend.cpp

@@ -1,7 +1,12 @@
 #include <gtest/gtest.h>
 #include <nifake/nifake_client.h>
 #include <nifake/nifake_mock_library.h>
+#pragma warning(push)
+#pragma warning(disable : 4616)
+#pragma warning(disable : 4146)
+#pragma warning(disable : 4244)
 #include <nifake/nifake_service.h>
+#pragma warning(pop)
 
 #include <atomic>
 #include <memory>

source/tests/system/nidaqmx_driver_api_tests.cpp

@@ -1388,7 +1388,6 @@ TEST_F(NiDAQmxDriverApiTests, SetYInterceptAttribute_GetYInterceptAttribute_Retu
 TEST_F(NiDAQmxDriverApiTests, SetPreScaledUnits_GetPreScaledUnits_ReturnsAttribute)
 {
   const auto SCALE_NAME = std::string("TestScale");
-  const auto Y_INTERCEPT = -3.0;
   auto scale_status = create_lin_scale(SCALE_NAME, 0.5);
   auto set_response = client::set_scale_attribute_int32(
       stub(),

source/tests/system/nidcpower_session_tests.cpp

@@ -101,7 +101,6 @@ TEST_F(NiDCPowerSessionTest, InitializeSessionWithDeviceAndSessionName_CreatesDr
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiDCPowerSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesDriverSession)
@@ -113,7 +112,6 @@ TEST_F(NiDCPowerSessionTest, InitializeSessionWithDeviceAndNoSessionName_Creates
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiDCPowerSessionTest, InitializeSessionWithoutDevice_ReturnsDriverError)

source/tests/system/nidigital_session_tests.cpp

@@ -66,7 +66,6 @@ TEST_F(NiDigitalSessionTest, InitializeSessionWithDeviceAndSessionName_CreatesDr
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiDigitalSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesDriverSession)
@@ -77,7 +76,6 @@ TEST_F(NiDigitalSessionTest, InitializeSessionWithDeviceAndNoSessionName_Creates
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiDigitalSessionTest, InitializedSession_CloseSession_ClosesDriverSession)

source/tests/system/nidmm_session_tests.cpp

@@ -57,23 +57,6 @@ class NiDmmSessionTest : public ::testing::Test {
     return status;
   }
 
-  std::string get_error_message(int error_status)
-  {
-    dmm::InitWithOptionsResponse init_response;
-    call_init_with_options(kResourceName, kDmmOptionsString, kSessionName, &init_response);
-    nidevice_grpc::Session session = init_response.vi();
-
-    ::grpc::ClientContext context;
-    dmm::GetErrorMessageRequest error_request;
-    error_request.mutable_vi()->set_name(session.name());
-    error_request.set_error_code(error_status);
-    dmm::GetErrorMessageResponse error_response;
-
-    ::grpc::Status status = GetStub()->GetErrorMessage(&context, error_request, &error_response);
-    EXPECT_TRUE(status.ok());
-    return error_response.error_message();
-  }
-
  private:
   DeviceServerInterface* device_server_;
   std::unique_ptr<::nidevice_grpc::Session> driver_session_;
@@ -88,7 +71,6 @@ TEST_F(NiDmmSessionTest, InitializeSessionWithDeviceAndSessionName_CreatesDriver
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiDmmSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesDriverSession)
@@ -99,7 +81,6 @@ TEST_F(NiDmmSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesDriv
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiDmmSessionTest, InitializedSession_CloseSession_ClosesDriverSession)

source/tests/system/nifgen_session_tests.cpp

@@ -82,7 +82,6 @@ TEST_F(NiFgenSessionTest, InitializeSessionWithDeviceAndSessionName_CreatesDrive
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiFgenSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesDriverSession)
@@ -93,7 +92,6 @@ TEST_F(NiFgenSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesDri
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiFgenSessionTest, InitializedSession_CloseSession_ClosesDriverSession)

source/tests/system/nirfmxspecan_session_tests.cpp

@@ -88,7 +88,6 @@ TEST_F(NiRFmxSpecAnSessionTest, InitializedSession_CloseSession_ClosesDriverSess
 
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, close_response.status());
-  EXPECT_THAT(init_response.error_message(), IsEmpty());
 }
 
 // Note: the error_message is included in the Init response because querying for errors
@@ -120,7 +119,6 @@ TEST_F(NiRFmxSpecAnSessionTest, InitWithErrorFromDriver_ReinitSuccessfully_Error
 
   EXPECT_EQ(::grpc::StatusCode::OK, status_two.error_code());
   EXPECT_THAT(status_two.error_message(), IsEmpty());
-  EXPECT_THAT(successful_init_response.error_message(), IsEmpty());
 }
 
 TEST_F(NiRFmxSpecAnSessionTest, InvalidSession_CloseSession_ReturnsInvalidSessionError)

source/tests/system/nirfsg_session_tests.cpp

@@ -72,7 +72,6 @@ TEST_F(NiRFSGSessionTest, InitializeSessionWithDeviceAndSessionName_CreatesDrive
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiRFSGSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesDriverSession)
@@ -83,7 +82,6 @@ TEST_F(NiRFSGSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesDri
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiRFSGSessionTest, InitializedSession_CloseSession_ClosesDriverSession)

source/tests/system/niscope_session_tests.cpp

@@ -78,7 +78,6 @@ TEST_F(NiScopeSessionTest, InitializeSessionWithDeviceAndSessionName_CreatesDriv
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiScopeSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesDriverSession)
@@ -89,7 +88,6 @@ TEST_F(NiScopeSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesDr
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiScopeSessionTest, InitializedSession_CloseSession_ClosesDriverSession)

source/tests/system/niswitch_session_tests.cpp

@@ -80,7 +80,6 @@ TEST_F(NiSwitchSessionTest, InitializeSessionWithDeviceAndSessionName_CreatesDri
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiSwitchSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesDriverSession)
@@ -91,7 +90,6 @@ TEST_F(NiSwitchSessionTest, InitializeSessionWithDeviceAndNoSessionName_CreatesD
   EXPECT_TRUE(status.ok());
   EXPECT_EQ(0, response.status());
   EXPECT_NE("", response.vi().name());
-  EXPECT_EQ("", response.error_message());
 }
 
 TEST_F(NiSwitchSessionTest, InitializedSession_CloseSession_ClosesDriverSession)

source/tests/system/nixnet_ethernet_driver_api_tests.cpp

@@ -116,7 +116,7 @@ TEST_F(NiXnetEthernetDriverApiTestsWithHardware, WriteFrameData_ReadFrameData_Va
   auto frame_out = read_frame_response.buffer()[0];
   // The frame that is read will contain mac address of the source on which the test is being run. For comparison purposes, we will be masking it.
   for (int i = 6; i < 12; i++) {
-    const_cast<char*>(frame_out.mutable_enet()->mutable_frame_data()->data())[i] = 0xFF;
+    const_cast<char*>(frame_out.mutable_enet()->mutable_frame_data()->data())[i] = 0xFFU;
   }
 
   EXPECT_EQ(frame->type(), frame_out.enet().type());

source/tests/system/nixnetsocket_driver_api_tests.cpp

@@ -219,15 +219,11 @@ class NiXnetSocketNoHardwareTests : public NiXnetSocketDriverApiTests {
 #define EXPECT_SUCCESS(response)               \
   if (1) {                                     \
     EXPECT_EQ(0, (response).status());         \
-    EXPECT_EQ("", (response).error_message()); \
-    EXPECT_EQ(0, (response).error_num());      \
   }
 
 #define EXPECT_SUCCESS_WITH_STATUS(expected_status, response) \
   if (1) {                                                    \
     EXPECT_EQ(expected_status, (response).status());          \
-    EXPECT_EQ("", (response).error_message());                \
-    EXPECT_EQ(0, (response).error_num());                     \
   }
 
 #define EXPECT_XNET_STATUS(error, response) \
@@ -242,8 +238,6 @@ class NiXnetSocketNoHardwareTests : public NiXnetSocketDriverApiTests {
 #define EXPECT_XNET_ERROR(expected_status, error, message, response) \
   if (1) {                                                           \
     EXPECT_EQ(expected_status, (response).status());                 \
-    EXPECT_EQ(error, (response).error_num());                        \
-    EXPECT_THAT((response).error_message(), HasSubstr(message));     \
   }
 
 #define EXPECT_INVALID_ARGUMENT_ERROR(response)                                                              \

source/tests/unit/ni_fake_non_ivi_service_tests.cpp

@@ -1776,7 +1776,6 @@ TEST_F(NiFakeNonIviServiceTests, InitWithNoError_DoesNotCallGetLatestError)
   InitResponse response;
   service_.Init(&context, &request, &response);
 
-  EXPECT_THAT(response.error_message(), IsEmpty());
   EXPECT_EQ(kDriverSuccess, response.status());
 }
 

source/tests/unit/ni_fake_service_tests.cpp

@@ -1,7 +1,12 @@
 #include <grpcpp/impl/grpc_library.h>
 #include <gtest/gtest.h>
 #include <nifake/nifake_mock_library.h>
+#pragma warning(push)
+#pragma warning(disable : 4616)
+#pragma warning(disable : 4146)
+#pragma warning(disable : 4244)
 #include <nifake/nifake_service.h>
+#pragma warning(pop)
 #include <nifake_extension/nifake_extension_mock_library.h>
 #include <nifake_extension/nifake_extension_service.h>
 #include <server/session_repository.h>