Don't suppress IO errors when writing files
* Close or flush streams explicitely so writing is not deferred * to catch errors in the right place * to avoid suppressing errors completely when writing would be deferred to the destructor invocation * Improve comments
This commit is contained in:
parent
a59a01cfe9
commit
7043c3d2a9
|
@ -10,7 +10,7 @@ set(META_APP_URL "https://github.com/${META_APP_AUTHOR}/${META_PROJECT_NAME}")
|
|||
set(META_APP_DESCRIPTION "C++ library for reading and writing MP4 (iTunes), ID3, Vorbis, Opus, FLAC and Matroska tags")
|
||||
set(META_VERSION_MAJOR 9)
|
||||
set(META_VERSION_MINOR 1)
|
||||
set(META_VERSION_PATCH 1)
|
||||
set(META_VERSION_PATCH 2)
|
||||
set(META_REQUIRED_CPP_UNIT_VERSION 1.14.0)
|
||||
set(META_ADD_DEFAULT_CPP_UNIT_TEST_APPLICATION ON)
|
||||
|
||||
|
|
|
@ -68,19 +68,21 @@ void restoreOriginalFileFromBackupFile(
|
|||
}
|
||||
// remove original file and restore backup
|
||||
std::remove(originalPath.c_str());
|
||||
if (std::rename(BasicFileInfo::pathForOpen(backupPath), BasicFileInfo::pathForOpen(originalPath))) {
|
||||
// can't rename/move the file (maybe backup dir on another partition) -> make a copy instead
|
||||
try {
|
||||
// need to open all streams again
|
||||
backupStream.exceptions(ios_base::failbit | ios_base::badbit);
|
||||
originalStream.exceptions(ios_base::failbit | ios_base::badbit);
|
||||
backupStream.open(backupPath, ios_base::in | ios_base::binary);
|
||||
originalStream.open(originalPath, ios_base::out | ios_base::binary);
|
||||
originalStream << backupStream.rdbuf();
|
||||
// TODO: callback for progress updates
|
||||
} catch (const std::ios_base::failure &failure) {
|
||||
throw std::ios_base::failure("Unable to restore original file from backup file \"" % backupPath % "\" after failure: " + failure.what());
|
||||
}
|
||||
if (std::rename(BasicFileInfo::pathForOpen(backupPath), BasicFileInfo::pathForOpen(originalPath)) == 0) {
|
||||
return;
|
||||
}
|
||||
// can't rename/move the file (maybe backup dir on another partition) -> make a copy instead
|
||||
try {
|
||||
// need to open all streams again
|
||||
backupStream.exceptions(ios_base::failbit | ios_base::badbit);
|
||||
originalStream.exceptions(ios_base::failbit | ios_base::badbit);
|
||||
backupStream.open(backupPath, ios_base::in | ios_base::binary);
|
||||
originalStream.open(originalPath, ios_base::out | ios_base::binary);
|
||||
originalStream << backupStream.rdbuf();
|
||||
originalStream.flush();
|
||||
// TODO: callback for progress updates
|
||||
} catch (const std::ios_base::failure &failure) {
|
||||
throw std::ios_base::failure("Unable to restore original file from backup file \"" % backupPath % "\" after failure: " + failure.what());
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -181,6 +183,7 @@ void createBackupFile(const std::string &backupDir, const std::string &originalP
|
|||
originalStream.open(BasicFileInfo::pathForOpen(originalPath), ios_base::in | ios_base::binary);
|
||||
// do the actual copying
|
||||
backupStream << originalStream.rdbuf();
|
||||
backupStream.flush();
|
||||
// streams are closed in the next try-block
|
||||
// TODO: callback for progress updates
|
||||
} catch (const std::ios_base::failure &failure) {
|
||||
|
|
|
@ -1845,7 +1845,7 @@ void MatroskaContainer::internalMakeFile(Diagnostics &diag, AbortableProgressFee
|
|||
}
|
||||
}
|
||||
|
||||
// flush output stream
|
||||
// prevent deferring final write operations (to catch and handle possible errors here)
|
||||
outputStream.flush();
|
||||
|
||||
// handle errors (which might have been occurred after renaming/creating backup file)
|
||||
|
|
|
@ -1544,6 +1544,8 @@ void MediaFileInfo::makeMp3File(Diagnostics &diag, AbortableProgressFeedback &pr
|
|||
diag.emplace_back(DiagLevel::Warning, "Unable to write ID3v1 tag.", context);
|
||||
}
|
||||
}
|
||||
// prevent deferring final write operations (to catch and handle possible errors here)
|
||||
stream().flush();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
@ -1747,13 +1749,14 @@ void MediaFileInfo::makeMp3File(Diagnostics &diag, AbortableProgressFeedback &pr
|
|||
reportPathChanged(saveFilePath());
|
||||
m_saveFilePath.clear();
|
||||
}
|
||||
// stream is useless for further usage because it is write-only
|
||||
// prevent deferring final write operations (to catch and handle possible errors here); stream is useless for further
|
||||
// usage anyways because it is write-only
|
||||
outputStream.close();
|
||||
} else {
|
||||
const auto newSize = static_cast<std::uint64_t>(outputStream.tellp());
|
||||
if (newSize < size()) {
|
||||
// file is smaller after the modification -> truncate
|
||||
// -> close stream before truncating
|
||||
// -> prevent deferring final write operations
|
||||
outputStream.close();
|
||||
// -> truncate file
|
||||
if (truncate(BasicFileInfo::pathForOpen(path()), static_cast<streamoff>(newSize)) == 0) {
|
||||
|
@ -1762,7 +1765,10 @@ void MediaFileInfo::makeMp3File(Diagnostics &diag, AbortableProgressFeedback &pr
|
|||
diag.emplace_back(DiagLevel::Critical, "Unable to truncate the file.", context);
|
||||
}
|
||||
} else {
|
||||
// file is longer after the modification -> just report new size
|
||||
// file is longer after the modification
|
||||
// -> prevent deferring final write operations (to catch and handle possible errors here)
|
||||
outputStream.flush();
|
||||
// -> report new size
|
||||
reportSizeChanged(newSize);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -881,7 +881,7 @@ calculatePadding:
|
|||
}
|
||||
}
|
||||
|
||||
// flush output stream
|
||||
// prevent deferring final write operations (to catch and handle possible errors here)
|
||||
outputStream.flush();
|
||||
|
||||
// handle errors (which might have been occurred after renaming/creating backup file)
|
||||
|
|
|
@ -580,6 +580,9 @@ void OggContainer::internalMakeFile(Diagnostics &diag, AbortableProgressFeedback
|
|||
OggPage::updateChecksum(fileInfo().stream(), offset);
|
||||
}
|
||||
|
||||
// prevent deferring final write operations (to catch and handle possible errors here)
|
||||
fileInfo().stream().flush();
|
||||
|
||||
// clear iterator
|
||||
m_iterator.clear(fileInfo().stream(), startOffset(), fileInfo().size());
|
||||
|
||||
|
|
Loading…
Reference in New Issue