diff --git a/lib-src/libflac/src/libFLAC/stream_encoder.c b/lib-src/libflac/src/libFLAC/stream_encoder.c index 45bdb252b..cc6b16166 100644 --- a/lib-src/libflac/src/libFLAC/stream_encoder.c +++ b/lib-src/libflac/src/libFLAC/stream_encoder.c @@ -1469,7 +1469,8 @@ FLAC_API FLAC__bool FLAC__stream_encoder_finish(FLAC__StreamEncoder *encoder) if(0 != encoder->private_->file) { if(encoder->private_->file != stdout) - fclose(encoder->private_->file); + error = fflush(encoder->private_->file) || + fclose(encoder->private_->file); encoder->private_->file = 0; } diff --git a/src/FFmpeg.cpp b/src/FFmpeg.cpp index eecb44587..380070004 100644 --- a/src/FFmpeg.cpp +++ b/src/FFmpeg.cpp @@ -209,10 +209,17 @@ int ufile_close(AVIOContext *pb) { std::unique_ptr f{ (wxFile *)pb->opaque }; - if (f) - f->Close(); + bool success = true; + if (f) { + success = f->Flush() && f->Close(); + pb->opaque = nullptr; + } - return 0; + // We're not certain that a close error is for want of space, but we'll + // guess that + return success ? 0 : -ENOSPC; + + // Implicitly destroy the wxFile object here } // Open a file with a (possibly) Unicode filename diff --git a/src/FFmpeg.h b/src/FFmpeg.h index ebdcfd0b3..c3807382f 100644 --- a/src/FFmpeg.h +++ b/src/FFmpeg.h @@ -928,6 +928,18 @@ private: // Deleter adaptor for functions like av_free that take a pointer template struct AV_Deleter { + inline R operator() (T* p) const + { + R result{}; + if (p) + result = Fn(p); + return result; + } +}; + +// Specialization of previous for void return +template +struct AV_Deleter { inline void operator() (T* p) const { if (p) @@ -959,9 +971,19 @@ using AVCodecContextHolder = std::unique_ptr< using AVDictionaryCleanup = std::unique_ptr< AVDictionary*, AV_Deleter >; -using UFileHolder = std::unique_ptr< +struct UFileHolder : public std::unique_ptr< AVIOContext, AV_Deleter ->; +> +{ + // Close explicitly, not ignoring return values. + int close() + { + auto result = get_deleter() ( get() ); + release(); + return result; + } +}; + template using AVMallocHolder = std::unique_ptr< T, AV_Deleter >; diff --git a/src/FileFormats.cpp b/src/FileFormats.cpp index e1fc3c59a..badfb5fd1 100644 --- a/src/FileFormats.cpp +++ b/src/FileFormats.cpp @@ -288,7 +288,7 @@ OSType sf_header_mactype(int format) ODLock libSndFileMutex; -void SFFileCloser::operator() (SNDFILE *sf) const +int SFFileCloser::operator() (SNDFILE *sf) const { auto err = SFCall(sf_close, sf); if (err) { @@ -299,4 +299,5 @@ void SFFileCloser::operator() (SNDFILE *sf) const (_("Error (file may not have been written): %s"), buffer)); } + return err; } diff --git a/src/FileFormats.h b/src/FileFormats.h index e2361f688..08dc57b37 100644 --- a/src/FileFormats.h +++ b/src/FileFormats.h @@ -127,7 +127,16 @@ inline R SFCall(F fun, Args&&... args) } //RAII for SNDFILE* -struct SFFileCloser { void operator () (SNDFILE*) const; }; -using SFFile = std::unique_ptr; +struct SFFileCloser { int operator () (SNDFILE*) const; }; +struct SFFile : public std::unique_ptr +{ + // Close explicitly, not ignoring return values. + int close() + { + auto result = get_deleter() ( get() ); + release(); + return result; + } +}; #endif diff --git a/src/FileIO.cpp b/src/FileIO.cpp index bc59040df..ecaca8b54 100644 --- a/src/FileIO.cpp +++ b/src/FileIO.cpp @@ -52,11 +52,18 @@ bool FileIO::IsOpened() return mOpen; } -void FileIO::Close() +bool FileIO::Close() { - mOutputStream.reset(); + bool success = true; + if (mOutputStream) { + // mOutputStream->Sync() returns void! Rrr! + success = mOutputStream->GetFile()->Flush() && + mOutputStream->Close(); + mOutputStream.reset(); + } mInputStream.reset(); mOpen = false; + return success; } wxInputStream & FileIO::Read(void *buf, size_t size) diff --git a/src/FileIO.h b/src/FileIO.h index 5200503df..404638aa3 100644 --- a/src/FileIO.h +++ b/src/FileIO.h @@ -32,7 +32,7 @@ class FileIO bool IsOpened(); - void Close(); + bool Close(); wxInputStream & Read(void *buffer, size_t size); wxOutputStream & Write(const void *buffer, size_t size); @@ -41,7 +41,7 @@ class FileIO wxString mName; FileIOMode mMode; std::unique_ptr mInputStream; - std::unique_ptr mOutputStream; + std::unique_ptr mOutputStream; bool mOpen; }; diff --git a/src/export/ExportFFmpeg.cpp b/src/export/ExportFFmpeg.cpp index a01a11b20..118d082fb 100644 --- a/src/export/ExportFFmpeg.cpp +++ b/src/export/ExportFFmpeg.cpp @@ -945,6 +945,12 @@ ProgressResult ExportFFmpeg::Export(AudacityProject *project, if ( !Finalize() ) // Finalize makes its own messages return ProgressResult::Cancelled; + if ( mUfileCloser.close() != 0 ) { + // TODO: more precise message + AudacityMessageBox(_("Unable to export")); + return ProgressResult::Cancelled; + } + return updateResult; } diff --git a/src/export/ExportFLAC.cpp b/src/export/ExportFLAC.cpp index 0c4647a3e..efc2d38dc 100644 --- a/src/export/ExportFLAC.cpp +++ b/src/export/ExportFLAC.cpp @@ -335,10 +335,13 @@ ProgressResult ExportFLAC::Export(AudacityProject *project, mMetadata.reset(); auto cleanup2 = finally( [&] { + if (!(updateResult == ProgressResult::Success || + updateResult == ProgressResult::Stopped)) { #ifndef LEGACY_FLAC - f.Detach(); // libflac closes the file + f.Detach(); // libflac closes the file #endif - encoder.finish(); + encoder.finish(); + } } ); const WaveTrackConstArray waveTracks = @@ -389,6 +392,20 @@ ProgressResult ExportFLAC::Export(AudacityProject *project, } } + if (updateResult == ProgressResult::Success || + updateResult == ProgressResult::Stopped) { +#ifndef LEGACY_FLAC + f.Detach(); // libflac closes the file +#endif + if (!encoder.finish()) + // Do not reassign updateResult, see cleanup2 + return ProgressResult::Failed; +#ifdef LEGACY_FLAC + if (!f.Flush() || !f.Close()) + return ProgressResult::Failed; +#endif + } + return updateResult; } diff --git a/src/export/ExportMP2.cpp b/src/export/ExportMP2.cpp index 4fd603ea5..0b790962b 100644 --- a/src/export/ExportMP2.cpp +++ b/src/export/ExportMP2.cpp @@ -330,7 +330,11 @@ ProgressResult ExportMP2::Export(AudacityProject *project, return ProgressResult::Cancelled; } - outFile.Close(); + if ( !outFile.Close() ) { + // TODO: more precise message + AudacityMessageBox(_("Unable to export")); + return ProgressResult::Cancelled; + } return updateResult; } diff --git a/src/export/ExportMP3.cpp b/src/export/ExportMP3.cpp index 8c0b2a60f..667db2de7 100644 --- a/src/export/ExportMP3.cpp +++ b/src/export/ExportMP3.cpp @@ -854,7 +854,7 @@ public: int FinishStream(unsigned char outbuffer[]); void CancelEncoding(); - void PutInfoTag(wxFFile & f, wxFileOffset off); + bool PutInfoTag(wxFFile & f, wxFileOffset off); private: @@ -1393,17 +1393,21 @@ void MP3Exporter::CancelEncoding() mEncoding = false; } -void MP3Exporter::PutInfoTag(wxFFile & f, wxFileOffset off) +bool MP3Exporter::PutInfoTag(wxFFile & f, wxFileOffset off) { if (mGF) { if (mInfoTagLen > 0) { // FIXME: TRAP_ERR Seek and writ ein MP3 exporter could fail. - f.Seek(off, wxFromStart); - f.Write(mInfoTagBuf, mInfoTagLen); + if ( !f.Seek(off, wxFromStart)) + return false; + if (mInfoTagLen > f.Write(mInfoTagBuf, mInfoTagLen)) + return false; } #if defined(__WXMSW__) else if (beWriteInfoTag) { - f.Flush(); + if ( !f.Flush() ) + return false; + // PRL: What is the correct error check on the return value? beWriteInfoTag(mGF, OSOUTPUT(f.GetName())); mGF = NULL; } @@ -1413,7 +1417,10 @@ void MP3Exporter::PutInfoTag(wxFFile & f, wxFileOffset off) } } - f.SeekEnd(); + if ( !f.SeekEnd() ) + return false; + + return true; } #if defined(__WXMSW__) @@ -1801,7 +1808,7 @@ ProgressResult ExportMP3::Export(AudacityProject *project, bool endOfFile; id3len = AddTags(project, id3buffer, &endOfFile, metadata); if (id3len && !endOfFile) { - if (id3len < outFile.Write(id3buffer.get(), id3len)) { + if (id3len > outFile.Write(id3buffer.get(), id3len)) { // TODO: more precise message AudacityMessageBox(_("Unable to export")); return ProgressResult::Cancelled; @@ -1887,7 +1894,7 @@ ProgressResult ExportMP3::Export(AudacityProject *project, break; } - if (bytes < outFile.Write(buffer.get(), bytes)) { + if (bytes > outFile.Write(buffer.get(), bytes)) { // TODO: more precise message AudacityMessageBox(_("Unable to export")); updateResult = ProgressResult::Cancelled; @@ -1909,7 +1916,7 @@ ProgressResult ExportMP3::Export(AudacityProject *project, } if (bytes > 0) { - if (bytes < outFile.Write(buffer.get(), bytes)) { + if (bytes > outFile.Write(buffer.get(), bytes)) { // TODO: more precise message AudacityMessageBox(_("Unable to export")); return ProgressResult::Cancelled; @@ -1918,7 +1925,7 @@ ProgressResult ExportMP3::Export(AudacityProject *project, // Write ID3 tag if it was supposed to be at the end of the file if (id3len > 0 && endOfFile) { - if (bytes < outFile.Write(id3buffer.get(), id3len)) { + if (bytes > outFile.Write(id3buffer.get(), id3len)) { // TODO: more precise message AudacityMessageBox(_("Unable to export")); return ProgressResult::Cancelled; @@ -1931,9 +1938,9 @@ ProgressResult ExportMP3::Export(AudacityProject *project, // // Also, if beWriteInfoTag() is used, mGF will no longer be valid after // this call, so do not use it. - exporter.PutInfoTag(outFile, pos); - - if (!outFile.Close()) { + if (!exporter.PutInfoTag(outFile, pos) || + !outFile.Flush() || + !outFile.Close()) { // TODO: more precise message AudacityMessageBox(_("Unable to export")); return ProgressResult::Cancelled; diff --git a/src/export/ExportOGG.cpp b/src/export/ExportOGG.cpp index 0a71fb2e5..51b420cb7 100644 --- a/src/export/ExportOGG.cpp +++ b/src/export/ExportOGG.cpp @@ -359,7 +359,11 @@ ProgressResult ExportOGG::Export(AudacityProject *project, } } - outFile.Close(); + if ( !outFile.Close() ) { + updateResult = ProgressResult::Cancelled; + // TODO: more precise message + AudacityMessageBox(_("Unable to export")); + } return updateResult; } diff --git a/src/export/ExportPCM.cpp b/src/export/ExportPCM.cpp index 50641950a..44c819181 100644 --- a/src/export/ExportPCM.cpp +++ b/src/export/ExportPCM.cpp @@ -530,7 +530,7 @@ ProgressResult ExportPCM::Export(AudacityProject *project, // Install the WAV metata in a "LIST" chunk at the end of the file if (updateResult == ProgressResult::Success || - updateResult == ProgressResult::Stopped) + updateResult == ProgressResult::Stopped) { if ((sf_format & SF_FORMAT_TYPEMASK) == SF_FORMAT_WAV || (sf_format & SF_FORMAT_TYPEMASK) == SF_FORMAT_WAVEX) { if (!AddStrings(project, sf.get(), metadata, sf_format)) { @@ -538,6 +538,12 @@ ProgressResult ExportPCM::Export(AudacityProject *project, AudacityMessageBox(_("Unable to export")); return ProgressResult::Cancelled; } + } + if (0 != sf.close()) { + // TODO: more precise message + AudacityMessageBox(_("Unable to export")); + return ProgressResult::Cancelled; + } } } @@ -545,6 +551,7 @@ ProgressResult ExportPCM::Export(AudacityProject *project, updateResult == ProgressResult::Stopped) if (((sf_format & SF_FORMAT_TYPEMASK) == SF_FORMAT_AIFF) || ((sf_format & SF_FORMAT_TYPEMASK) == SF_FORMAT_WAV)) + // Note: file has closed, and gets reopened and closed again here: if (!AddID3Chunk(fName, metadata, sf_format) ) { // TODO: more precise message AudacityMessageBox(_("Unable to export")); @@ -845,6 +852,9 @@ bool ExportPCM::AddID3Chunk(wxString fName, const Tags *tags, int sf_format) if (4 != f.Write(&sz, 4)) return false; + if (!f.Flush()) + return false; + if (!f.Close()) return false; }