[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1028: MINIFICPP-1507 convert InputStream::read to size_t

[GitBox]

fgerlits commented on a change in pull request #1028:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1028#discussion_r595891399



##########
File path: libminifi/src/sitetosite/SiteToSiteClient.cpp
##########
@@ -31,47 +31,37 @@ namespace sitetosite {
 
 int SiteToSiteClient::readResponse(const std::shared_ptr<Transaction>& 
/*transaction*/, RespondCode &code, std::string &message) {
   uint8_t firstByte;
-
-  int ret = peer_->read(firstByte);
-
-  if (ret <= 0 || firstByte != CODE_SEQUENCE_VALUE_1)
+  auto ret = peer_->read(firstByte);
+  if (ret == 0 || ret == static_cast<size_t>(-1) || firstByte != 
CODE_SEQUENCE_VALUE_1)
     return -1;
 
   uint8_t secondByte;
-
   ret = peer_->read(secondByte);
-
-  if (ret <= 0 || secondByte != CODE_SEQUENCE_VALUE_2)
+  if (ret == 0 || ret == static_cast<size_t>(-1) || secondByte != 
CODE_SEQUENCE_VALUE_2)
     return -1;
 
   uint8_t thirdByte;
-
   ret = peer_->read(thirdByte);
-
-  if (ret <= 0)
-    return ret;
+  if (ret == 0 || ret == static_cast<size_t>(-1))
+    return gsl::narrow_cast<int>(ret);
 
   code = (RespondCode) thirdByte;
-
   RespondCodeContext *resCode = this->getRespondCodeContext(code);
-
-  if (resCode == NULL) {
-    // Not a valid respond code
+  if (!resCode) {
     return -1;
   }
   if (resCode->hasDescription) {
     ret = peer_->read(message);
-    if (ret <= 0)
+    if (ret != static_cast<size_t>(-1))

Review comment:
       missing `== 0 ||`

##########
File path: libminifi/src/io/InputStream.cpp
##########
@@ -30,42 +30,43 @@ namespace nifi {
 namespace minifi {
 namespace io {
 
-int InputStream::read(std::vector<uint8_t>& buffer, int len) {
-  if (buffer.size() < gsl::narrow<size_t>(len)) {
+size_t InputStream::read(std::vector<uint8_t>& buffer, size_t len) {
+  if (buffer.size() < len) {
     buffer.resize(len);
   }
-  int ret = read(buffer.data(), len);
-  buffer.resize((std::max)(ret, 0));
+  const auto ret = read(buffer.data(), len);
+  if (ret == static_cast<size_t>(-1)) return ret;
+  buffer.resize((std::max)(ret, size_t{0}));
   return ret;
 }
 
-int InputStream::read(bool &value) {
+size_t InputStream::read(bool &value) {
   uint8_t buf = 0;
 
   if (read(&buf, 1) != 1) {
-    return -1;
+    return static_cast<size_t>(-1);
   }
   value = buf;
   return 1;
 }
 
-int InputStream::read(utils::Identifier &value) {
+size_t InputStream::read(utils::Identifier &value) {
   std::string uuidStr;
-  int ret = read(uuidStr);
+  const auto ret = read(uuidStr);
   if (ret < 0) {
     return ret;
   }
   auto optional_uuid = utils::Identifier::parse(uuidStr);
   if (!optional_uuid) {
-    return -1;
+    return static_cast<size_t>(-1);
   }
   value = optional_uuid.value();
   return ret;
 }
 
-int InputStream::read(std::string &str, bool widen) {
+size_t InputStream::read(std::string &str, bool widen) {
   uint32_t len = 0;
-  int ret = 0;
+  size_t ret = 0;
   if (!widen) {
     uint16_t shortLength = 0;
     ret = read(shortLength);

Review comment:
       I can't comment at line 78, but it needs to be updated from `<= 0` to 
`== 0 || == static_cast(-1)`, too

##########
File path: thirdparty/google-styleguide/run_linter.sh
##########
@@ -40,5 +40,7 @@ done
 
 HEADERS=`find $INCLUDE_DIRS -name '*.h' | sort | uniq | tr '\n' ' '`
 SOURCES=`find $SOURCE_DIRS -name  '*.cpp' | sort | uniq | tr '\n' ' '`
-REPOSITORY="$(realpath --physical "$(dirname "$0")/../..")"
+# this realpath alternative should work on mac
+alias prealpath="python -c 'import os, sys; 
print(os.path.realpath(sys.argv[1]))'"

Review comment:
       where is this alias used?




----------------------------------------------------------------
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.

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