[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1348: MINIFICPP-1833 - Integrate misc and modernize clang-tidy checks in CI

[GitBox <gi...@apache.org>]

szaszm commented on code in PR #1348:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1348#discussion_r893699308


##########
libminifi/test/unit/EnvironmentUtilsTests.cpp:
##########
@@ -117,6 +117,9 @@ TEST_CASE("multithreaded environment manipulation", "[getenv][setenv][unsetenv]"
               utils::Environment::unsetEnvironmentVariable(env_name.c_str());
               break;
             }
+          default: {
+            throw std::runtime_error("Operation value must be modulo 3");

Review Comment:
   I think this should terminate. If it ever happens in the future, it's a programming error.



##########
libminifi/src/c2/ControllerSocketProtocol.cpp:
##########
@@ -237,6 +233,8 @@ void ControllerSocketProtocol::initialize(core::controller::ControllerServicePro
           }
         }
         break;
+        default:
+          throw std::runtime_error("Unhandled operation: " + std::to_string(head));

Review Comment:
   Who catches this?



##########
extensions/librdkafka/KafkaConnection.cpp:
##########
@@ -125,11 +121,9 @@ void KafkaConnection::logCallback(const rd_kafka_t* rk, int level, const char* /
     case 7:  // LOG_DEBUG
       core::logging::LOG_DEBUG(logger) << buf;
       break;
+    default:
+      throw std::runtime_error("Unknown log level: " + std::to_string(level));

Review Comment:
   Again, I'd check if this makes sense. Terminate instead, if it's a programming error to have it happen?



##########
extensions/opc/src/opc.cpp:
##########
@@ -550,7 +546,7 @@ std::string OPCDateTime2String(UA_DateTime raw_date) {
 
   int sz = snprintf(charBuf.data(), charBuf.size(), "%02hu-%02hu-%04hu %02hu:%02hu:%02hu.%03hu", dts.day, dts.month, dts.year, dts.hour, dts.min, dts.sec, dts.milliSec);
 
-  return std::string(charBuf.data(), sz);
+  return {charBuf.data(), static_cast<std::size_t>(sz)};

Review Comment:
   that `static_cast` should be `gsl::narrow` instead, possibly with checking for negative values.



##########
extensions/coap/protocols/CoapC2Protocol.cpp:
##########
@@ -186,29 +181,30 @@ minifi::c2::Operation CoapProtocol::getOperation(int type) const {
       return minifi::c2::Operation::PAUSE;
     case 9:
       return minifi::c2::Operation::RESUME;
+    default:
+      return minifi::c2::Operation::ACKNOWLEDGE;

Review Comment:
   Let's exclude this from the domain of the function and have the default case terminate. Or if you don't want to make the callers check, a wide contract could work as well (throwing on invalid values).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org