adamdebreceni commented on code in PR #1648:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1648#discussion_r1325932017
##########
extensions/http-curl/client/HTTPClient.cpp:
##########
@@ -328,18 +325,15 @@ void HTTPClient::setRequestHeader(std::string key,
std::optional<std::string> va
}
namespace {
-struct CurlSListFreeAll {
- void operator()(struct curl_slist* slist) const {
- curl_slist_free_all(slist);
- }
-};
+using CurlSlistDeleter = decltype([](struct curl_slist* slist) {
curl_slist_free_all(slist); });
-std::unique_ptr<struct curl_slist, CurlSListFreeAll> getCurlSList(const
std::unordered_map<std::string, std::string>& request_headers) {
+std::unique_ptr<struct curl_slist, CurlSlistDeleter> toCurlSlist(const
std::unordered_map<std::string, std::string>& request_headers) {
curl_slist* new_list = nullptr;
+ const auto guard = gsl::finally([&new_list]() {
curl_slist_free_all(new_list); });
for (const auto& [header_key, header_value] : request_headers)
new_list = curl_slist_append(new_list,
utils::StringUtils::join_pack(header_key, ": ", header_value).c_str());
Review Comment:
we could eliminate the need for the guard and the raw pointer, we also did
not handle the error case of `curl_slist_append` returning a nullptr
from [curl.se](https://curl.se/libcurl/c/curl_slist_append.html)
`A null pointer is returned if anything went wrong, otherwise the new list
pointer is returned. To avoid overwriting an existing non-empty list on
failure, the new list should be returned to a temporary variable which can be
tested for NULL before updating the original list pointer. `
```suggestion
for (const auto& [header_key, header_value] : request_headers)
std::unique_ptr<struct curl_slist, CurlSlistDeleter>
appended_list{curl_slist_append(new_list.get(),
utils::StringUtils::join_pack(header_key, ": ", header_value).c_str())};
if (!appended_list) {
throw std::runtime_error("Failed to append to slist");
}
new_list.release();
new_list = std::move(appended_list);
```
--
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]
