Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Commit

Permalink
Fixed bug fail ostree_object_test
Browse files Browse the repository at this point in the history
Signed-off-by: Zee314159 <[email protected]>
  • Loading branch information
Zee314159 committed Aug 30, 2019
1 parent 79b7feb commit 915e9cd
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 13 deletions.
5 changes: 1 addition & 4 deletions src/sota_tools/ostree_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ OSTreeObject::OSTreeObject(const OSTreeRepo &repo, const std::string &object_nam
refcount_(0),
is_on_server_(PresenceOnServer::kObjectStateUnknown),
curl_handle_(nullptr),
form_post_(nullptr),
headers_(nullptr) {
if (!boost::filesystem::is_regular_file(file_path_)) {
throw std::runtime_error(file_path_.native() + " is not a valid OSTree repo.");
Expand Down Expand Up @@ -240,7 +239,7 @@ void OSTreeObject::Upload(const TreehubServer &push_target, CURLM *curl_multi_ha
FILE *fd;
struct stat file_info {};
fd = fopen(file_path_.c_str(), "rb");
if (!fd) {
if (fd == nullptr) {

This comment has been minimized.

Copy link
@mike-sul

mike-sul Aug 30, 2019

Collaborator

I am not sure if I see where this fd is closed in case if the file is successfully opened.

LOG_ERROR << "Could not open file: " << file_path_.c_str();
return;

This comment has been minimized.

Copy link
@mike-sul

mike-sul Aug 30, 2019

Collaborator

This return looks too early to me, as far as I understand the code logic it should set some error context here (e.g. UploadError(pool, rescode); and/or PresenceError(pool, rescode); ) as well as the curl stuff clean up.

This comment has been minimized.

Copy link
@Zee314159

Zee314159 Aug 30, 2019

Author Contributor

yeah, totally makes sense!

} else {
Expand Down Expand Up @@ -330,8 +329,6 @@ void OSTreeObject::CurlDone(CURLM *curl_multi_handle, RequestPool &pool) {
}

} else if (current_operation_ == CurrentOp::kOstreeObjectUploading) {
curl_formfree(form_post_);
form_post_ = nullptr;
// Sanity-check the handle's URL to make sure it contains the expected
// object hash.
if (url == nullptr || strstr(url, object_name_.c_str()) == nullptr) {
Expand Down
1 change: 0 additions & 1 deletion src/sota_tools/ostree_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ class OSTreeObject {

std::stringstream http_response_;
CURL* curl_handle_;
struct curl_httppost* form_post_;
curl_slist* headers_;
std::list<parentref> parents_;
std::list<OSTreeObject::ptr> children_;
Expand Down
8 changes: 0 additions & 8 deletions src/sota_tools/ostree_object_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,6 @@ TEST(OstreeObject, UploadDryRun) {
EXPECT_EQ(object->is_on_server_, PresenceOnServer::kObjectPresent);
// This currently does not get reset.
EXPECT_EQ(object->current_operation_, CurrentOp::kOstreeObjectPresenceCheck);
// This should not get allocated.
EXPECT_EQ(object->form_post_, nullptr);
}

/* Detect curl misconfiguration.
Expand All @@ -128,10 +126,6 @@ TEST(OstreeObject, UploadFail) {
object->Upload(push_server, nullptr, RunMode::kDefault);
EXPECT_EQ(object->is_on_server_, PresenceOnServer::kObjectStateUnknown);
EXPECT_EQ(object->current_operation_, CurrentOp::kOstreeObjectUploading);
// This currently will get allocated and we need to free it.
EXPECT_NE(object->form_post_, nullptr);
curl_formfree(object->form_post_);
object->form_post_ = nullptr;
}

/* Upload missing OSTree objects to destination repository. */
Expand Down Expand Up @@ -184,8 +178,6 @@ TEST(OstreeObject, UploadSuccess) {
curl_easy_getinfo(h->curl_handle_, CURLINFO_RESPONSE_CODE, &rescode);
EXPECT_EQ(rescode, 200);

curl_formfree(h->form_post_);
h->form_post_ = nullptr;
curl_multi_remove_handle(multi, h->curl_handle_);
curl_easy_cleanup(h->curl_handle_);
h->curl_handle_ = nullptr;
Expand Down

0 comments on commit 915e9cd

Please sign in to comment.