Skip to content

Commit

Permalink
pw_transfer: Fix File Transfer Handler
Browse files Browse the repository at this point in the history
The transfer failed with two separate issues
that this CL fixes:
1. Renaming/Copy fails when the full path
 to the target file does not exist.
2. Rename fails when tmp file and target file
are not on the same file system.

Change-Id: I964cb3a221ec0aed59534fc23b53651f1babac57
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/109811
Reviewed-by: Armando Montanez <amontanez@google.com>
Commit-Queue: Moussa Traore <traorem@google.com>
  • Loading branch information
traoremp authored and CQ Bot Account committed Sep 30, 2022
1 parent 2b3de52 commit b315b31
Showing 1 changed file with 72 additions and 13 deletions.
85 changes: 72 additions & 13 deletions pw_transfer/atomic_file_transfer_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,59 @@

namespace pw::transfer {

namespace {
// Linux Error code for Cross-Device Link Error.
constexpr auto CROSS_DEVICE_LINK_ERROR = 18;

pw::Status EnsureDirectoryExists(std::string_view filepath) {
const auto target_directory = std::filesystem::path{filepath}.parent_path();
return std::filesystem::exists(target_directory) ||
std::filesystem::create_directories(target_directory)
? pw::OkStatus()
: pw::Status::Internal();
}

// Copy file and remove on succes.
// If the copy fails, the file `input_target` is not removed.
pw::Status CopyFile(const std::string_view input_target,
const std::string_view output_target) {
auto err = std::error_code{};
std::filesystem::copy(input_target,
output_target,
std::filesystem::copy_options::overwrite_existing,
err);
if (err) {
PW_LOG_ERROR("Error with status code: %d (%s) during copy of file %s",
err.value(),
err.message().c_str(),
input_target.data());
return pw::Status::Internal();
}
PW_LOG_INFO("Successfully copied the file.");
if (!std::filesystem::remove(input_target)) {
PW_LOG_WARN("Failed to remove tmp file %s", input_target.data());
}
return pw::OkStatus();
}

// Uses the same approach as unix `mv` command. First try to rename. If we get
// a cross-device link error, copies then deletes input_target.
pw::Status RenameFile(const std::string_view input_target,
const std::string_view output_target) {
auto err = std::error_code{};
std::filesystem::rename(input_target, output_target, err);
if (err && err.value() == CROSS_DEVICE_LINK_ERROR) {
PW_LOG_INFO("%s[%d] during rename of file %s. Trying Copy/Remove.",
err.message().c_str(),
err.value(),
input_target.data());
return CopyFile(input_target, output_target);
}
return err ? pw::Status::Internal() : pw::OkStatus();
}

} // namespace

Status AtomicFileTransferHandler::PrepareRead() {
auto file_path = path_.c_str();
PW_LOG_DEBUG("Preparing read for file %s", file_path);
Expand All @@ -47,23 +100,29 @@ Status AtomicFileTransferHandler::PrepareWrite() {

Status AtomicFileTransferHandler::FinalizeWrite(Status status) {
std::get<stream::StdFileWriter>(stream_).Close();
const auto tmp_file = GetTempFilePath(path_);
auto temp_file_path = tmp_file.c_str();
auto file_path = path_.c_str();
if (!status.ok() || !std::filesystem::exists(temp_file_path) ||
std::filesystem::is_empty(temp_file_path)) {
PW_LOG_ERROR("Status not ok, remove file %s", temp_file_path);
auto tmp_file = GetTempFilePath(path_);
if (!status.ok() || !std::filesystem::exists(tmp_file) ||
std::filesystem::is_empty(tmp_file)) {
PW_LOG_ERROR("Transfer unsuccesful, attempt to remove temp file %s",
tmp_file.c_str());
// Remove temp file if transfer fails.
return std::filesystem::remove(tmp_file) ? status : Status::Aborted();
}
PW_LOG_DEBUG("Renaming file from: %s, to: %s", temp_file_path, file_path);
std::error_code err;
std::filesystem::rename(temp_file_path, file_path, err);
if (err) {
PW_LOG_ERROR("Error during renaming of file %s", temp_file_path);
return std::filesystem::remove(tmp_file) ? Status::Internal()
: Status::Aborted();

const auto directory_exists_status = EnsureDirectoryExists(path_);
if (!directory_exists_status.ok()) {
std::filesystem::remove(tmp_file);
return directory_exists_status;
}

PW_LOG_DEBUG(
"Copying file from: %s, to: %s", tmp_file.c_str(), path_.c_str());
const auto rename_status = RenameFile(tmp_file, path_);
if (!rename_status.ok()) {
std::filesystem::remove(tmp_file);
return rename_status;
}

PW_LOG_INFO("File transfer was successful.");
return OkStatus();
}
Expand Down

0 comments on commit b315b31

Please sign in to comment.