martinzink commented on code in PR #1616:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1616#discussion_r1275429341
##########
libminifi/src/io/StreamSlice.cpp:
##########
@@ -23,7 +23,8 @@ namespace org::apache::nifi::minifi::io {
StreamSlice::StreamSlice(std::shared_ptr<io::InputStream> stream, size_t
offset, size_t size) : stream_(std::move(stream)), slice_offset_(offset),
slice_size_(size) {
stream_->seek(slice_offset_);
if (stream_->size() < slice_offset_ + slice_size_)
- throw std::invalid_argument("StreamSlice is bigger than the Stream");
+ throw std::invalid_argument("StreamSlice is bigger than the Stream, Stream
size: " + std::to_string(stream_->size()) +
+ ", StreamSlice size: " + std::to_string(slice_size_) + ", offset: " +
std::to_string(slice_offset_));
Review Comment:
nitpicking but we could utilize fmt::format for a more compact and more
readable version
```suggestion
throw std::invalid_argument(fmt::format("StreamSlice is bigger than the
Stream, Stream size: {}, StreamSlice size: {}, offset: {}", stream_->size(),
slice_size_, slice_offset_));
```
##########
extensions/libarchive/MergeContent.cpp:
##########
@@ -213,18 +213,19 @@ bool MergeContent::processBin(core::ProcessContext
*context, core::ProcessSessio
});
}
- std::shared_ptr<core::FlowFile> merge_flow =
std::static_pointer_cast<FlowFileRecord>(session->create());
+ std::shared_ptr<core::FlowFile> merge_flow =
std::static_pointer_cast<FlowFileRecord>(session.create());
Review Comment:
I think it might be better to introduce a function to processSession that
checks if the flowfile is in a valid transferred state, and then let a RAII
take care of the removal instead. This way we dont have to keep in mind that we
have to remove it during the various early return paths.
something like this should do it
```
auto remove_merge_flow = gsl::finally([&](){
if (!session.hasBeenTransferred(*merge_flow))
session.remove(merge_flow);
});
```
whre the hasbeentransferred should check if the updated_relationships_ or
added_flowfiles_ maps contain a nonnull relationship* for the flowfile
##########
extensions/libarchive/MergeContent.cpp:
##########
@@ -246,27 +247,30 @@ bool MergeContent::processBin(core::ProcessContext
*context, core::ProcessSessio
mimeType = "application/zip";
} else {
logger_->log_error("Merge format not supported %s", mergeFormat_);
+ session.remove(merge_flow);
return false;
}
std::shared_ptr<core::FlowFile> mergeFlow;
Review Comment:
nitpicking and old code, but this seems unused
```suggestion
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
