arpadboda commented on a change in pull request #964:
URL: https://github.com/apache/nifi-minifi-cpp/pull/964#discussion_r555130531



##########
File path: libminifi/include/provenance/Provenance.h
##########
@@ -360,7 +361,7 @@ class ProvenanceEventRecord : public 
core::SerializableComponent {
   bool DeSerialize(const std::shared_ptr<core::SerializableComponent> &repo);
 
   uint64_t getEventTime(const uint8_t *buffer, const size_t bufferSize) {
-    int size = bufferSize > 72 ? 72 : bufferSize;
+    int size = gsl::narrow<int>(bufferSize > 72 ? 72 : bufferSize);

Review comment:
       I think this is simply an std::min written in complex form :) 

##########
File path: extensions/rocksdb-repos/FlowFileRepository.cpp
##########
@@ -15,12 +15,11 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include "FlowFileRecord.h"
 #include "FlowFileRepository.h"
 
-#include "rocksdb/options.h"
-#include "rocksdb/write_batch.h"
-#include "rocksdb/slice.h"
+#include <rocksdb/options.h>
+#include <rocksdb/write_batch.h>

Review comment:
       Nitpicking: these are 3rd party headers.
   As we fix the order here, I don't think these should be included before stl 
headers. 

##########
File path: libminifi/include/utils/StringUtils.h
##########
@@ -159,9 +160,9 @@ class StringUtils {
   }
 
   inline static std::string hex_ascii(const std::string& in) {
-    int len = in.length();
+    size_t len = in.length();

Review comment:
       I think we can simply remove the variable, it's only referenced in the 
header of the loop. 
   
   Completely unrelated, possible optimisation here: guess the lenght of the 
output and reserve place for the result string with that size to avoid 
reallocations caused by push_back. 




----------------------------------------------------------------
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:
[email protected]


Reply via email to