Improve OpenFileError so that it doesn't say "opening for read"

in one case where it should say "opening for read/write".

Also add some unit tests for ReelWriter.

10 files changed, 32 insertions, 18 deletions

diff --git a/src/lib/exceptions.cc b/src/lib/exceptions.cc
index 481d2e89d..bf474e3ea 100644
--- a/src/lib/exceptions.cc
+++ b/src/lib/exceptions.cc
@@ -27,10 +27,11 @@ using std::string;
 using std::runtime_error;
 
 /** @param f File that we were trying to open */
-OpenFileError::OpenFileError (boost::filesystem::path f, int error, bool reading)
+OpenFileError::OpenFileError (boost::filesystem::path f, int error, Mode mode)
 	: FileError (
 		String::compose (
-			reading ? _("could not open file %1 for reading (%2)") : _("could not open file %1 for writing (%2)"),
+			mode == READ_WRITE ? _("could not open file %1 for read/write (%2)") :
+			(mode == READ ? _("could not open file %1 for read (%2)") : _("could not open file %1 for write (%2)")),
 			f.string(),
 			error),
 		f
diff --git a/src/lib/exceptions.h b/src/lib/exceptions.h
index fe87ababc..766d3d8d5 100644
--- a/src/lib/exceptions.h
+++ b/src/lib/exceptions.h
@@ -103,11 +103,17 @@ public:
 class OpenFileError : public FileError
 {
 public:
+	enum Mode {
+		READ,
+		WRITE,
+		READ_WRITE
+	};
+
 	/** @param f File that we were trying to open.
 	 *  @param error Code of error that occurred.
-	 *  @param reading true if we were opening to read, false if opening to write.
+	 *  @param mode Mode that we tried to open the file in.
 	 */
-	OpenFileError (boost::filesystem::path f, int error, bool reading);
+	OpenFileError (boost::filesystem::path f, int error, Mode mode);
 };
 
 /** @class ReadFileError.
diff --git a/src/lib/ffmpeg.cc b/src/lib/ffmpeg.cc
index fb3437910..f26d1d2cf 100644
--- a/src/lib/ffmpeg.cc
+++ b/src/lib/ffmpeg.cc
@@ -128,7 +128,7 @@ FFmpeg::setup_general ()
 
 	int e = avformat_open_input (&_format_context, 0, 0, &options);
 	if (e < 0) {
-		throw OpenFileError (_ffmpeg_content->path(0).string(), e, true);
+		throw OpenFileError (_ffmpeg_content->path(0).string(), e, OpenFileError::READ);
 	}
 
 	if (avformat_find_stream_info (_format_context, 0) < 0) {
diff --git a/src/lib/ffmpeg_image_proxy.cc b/src/lib/ffmpeg_image_proxy.cc
index fa6cb288d..9c91d1d87 100644
--- a/src/lib/ffmpeg_image_proxy.cc
+++ b/src/lib/ffmpeg_image_proxy.cc
@@ -147,7 +147,7 @@ FFmpegImageProxy::image (optional<dcp::Size>) const
 	}
 	if (e < 0) {
 		if (_path) {
-			throw OpenFileError (_path->string(), e, true);
+			throw OpenFileError (_path->string(), e, OpenFileError::READ);
 		} else {
 			boost::throw_exception(DecodeError(String::compose(_("Could not decode image (%1)"), e)));
 		}
diff --git a/src/lib/file_group.cc b/src/lib/file_group.cc
index 942eb435d..3e8a7b79c 100644
--- a/src/lib/file_group.cc
+++ b/src/lib/file_group.cc
@@ -93,7 +93,7 @@ FileGroup::ensure_open_path (size_t p) const
 	_current_path = p;
 	_current_file = fopen_boost (_paths[_current_path], "rb");
 	if (_current_file == 0) {
-		throw OpenFileError (_paths[_current_path], errno, true);
+		throw OpenFileError (_paths[_current_path], errno, OpenFileError::READ);
 	}
 }
 
diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc
index b7ccc07ce..355fe5c3b 100644
--- a/src/lib/reel_writer.cc
+++ b/src/lib/reel_writer.cc
@@ -62,6 +62,7 @@ using dcp::raw_convert;
 
 int const ReelWriter::_info_size = 48;
 
+/** @param job Related job, or 0 */
 ReelWriter::ReelWriter (
 	shared_ptr<const Film> film, DCPTimePeriod period, shared_ptr<Job> job, int reel_index, int reel_count, optional<string> content_summary
 	)
@@ -97,7 +98,9 @@ ReelWriter::ReelWriter (
 		_film->internal_video_asset_dir() / _film->internal_video_asset_filename(_period)
 		);
 
-	job->sub (_("Checking existing image data"));
+	if (job) {
+		job->sub (_("Checking existing image data"));
+	}
 	_first_nonexistant_frame = check_existing_picture_asset ();
 
 	_picture_asset_writer = _picture_asset->start_write (
@@ -139,8 +142,9 @@ ReelWriter::write_frame_info (Frame frame, Eyes eyes, dcp::FrameInfo info) const
 	} else {
 		file = fopen_boost (info_file, "wb");
 	}
+
 	if (!file) {
-		throw OpenFileError (info_file, errno, read);
+		throw OpenFileError (info_file, errno, read ? OpenFileError::READ_WRITE : OpenFileError::WRITE);
 	}
 	dcpomatic_fseek (file, frame_info_position (frame, eyes), SEEK_SET);
 	checked_fwrite (&info.offset, sizeof (info.offset), file, info_file);
diff --git a/src/lib/reel_writer.h b/src/lib/reel_writer.h
index ae64c3ac7..741b0914c 100644
--- a/src/lib/reel_writer.h
+++ b/src/lib/reel_writer.h
@@ -30,6 +30,7 @@ class Film;
 class Job;
 class Font;
 class AudioBuffers;
+struct write_frame_info_test;
 
 namespace dcp {
 	class MonoPictureAsset;
@@ -89,6 +90,8 @@ public:
 
 private:
 
+	friend struct ::write_frame_info_test;
+
 	void write_frame_info (Frame frame, Eyes eyes, dcp::FrameInfo info) const;
 	long frame_info_position (Frame frame, Eyes eyes) const;
 	Frame check_existing_picture_asset ();
diff --git a/src/lib/util.cc b/src/lib/util.cc
index 340af1ea8..a17ac54f5 100644
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -471,7 +471,7 @@ digest_head_tail (vector<boost::filesystem::path> files, boost::uintmax_t size)
 	while (i < int64_t (files.size()) && to_do > 0) {
 		FILE* f = fopen_boost (files[i], "rb");
 		if (!f) {
-			throw OpenFileError (files[i].string(), errno, true);
+			throw OpenFileError (files[i].string(), errno, OpenFileError::READ);
 		}
 
 		boost::uintmax_t this_time = min (to_do, boost::filesystem::file_size (files[i]));
@@ -491,7 +491,7 @@ digest_head_tail (vector<boost::filesystem::path> files, boost::uintmax_t size)
 	while (i >= 0 && to_do > 0) {
 		FILE* f = fopen_boost (files[i], "rb");
 		if (!f) {
-			throw OpenFileError (files[i].string(), errno, true);
+			throw OpenFileError (files[i].string(), errno, OpenFileError::READ);
 		}
 
 		boost::uintmax_t this_time = min (to_do, boost::filesystem::file_size (files[i]));
diff --git a/src/lib/writer.cc b/src/lib/writer.cc
index d0f0825f1..8666d90c8 100644
--- a/src/lib/writer.cc
+++ b/src/lib/writer.cc
@@ -584,7 +584,7 @@ Writer::write_cover_sheet ()
 	boost::filesystem::path const cover = _film->file ("COVER_SHEET.txt");
 	FILE* f = fopen_boost (cover, "w");
 	if (!f) {
-		throw OpenFileError (cover, errno, false);
+		throw OpenFileError (cover, errno, OpenFileError::WRITE);
 	}
 
 	string text = Config::instance()->cover_sheet ();
diff --git a/src/wx/config_dialog.cc b/src/wx/config_dialog.cc
index 743953650..fecb55ea7 100644
--- a/src/wx/config_dialog.cc
+++ b/src/wx/config_dialog.cc
@@ -578,7 +578,7 @@ CertificateChainEditor::export_certificate ()
 		boost::filesystem::path path (wx_to_std(d->GetPath()));
 		FILE* f = fopen_boost (path, "w");
 		if (!f) {
-			throw OpenFileError (path, errno, false);
+			throw OpenFileError (path, errno, OpenFileError::WRITE);
 		}
 
 		string const s = j->certificate (true);
@@ -600,7 +600,7 @@ CertificateChainEditor::export_chain ()
 		boost::filesystem::path path (wx_to_std(d->GetPath()));
 		FILE* f = fopen_boost (path, "w");
 		if (!f) {
-			throw OpenFileError (path, errno, false);
+			throw OpenFileError (path, errno, OpenFileError::WRITE);
 		}
 
 		string const s = _get()->chain();
@@ -774,7 +774,7 @@ CertificateChainEditor::export_private_key ()
 		boost::filesystem::path path (wx_to_std(d->GetPath()));
 		FILE* f = fopen_boost (path, "w");
 		if (!f) {
-			throw OpenFileError (path, errno, false);
+			throw OpenFileError (path, errno, OpenFileError::WRITE);
 		}
 
 		string const s = _get()->key().get ();
@@ -867,7 +867,7 @@ KeysPage::export_decryption_chain_and_key ()
 		boost::filesystem::path path (wx_to_std(d->GetPath()));
 		FILE* f = fopen_boost (path, "w");
 		if (!f) {
-			throw OpenFileError (path, errno, false);
+			throw OpenFileError (path, errno, OpenFileError::WRITE);
 		}
 
 		string const chain = Config::instance()->decryption_chain()->chain();
@@ -902,7 +902,7 @@ KeysPage::import_decryption_chain_and_key ()
 
 		FILE* f = fopen_boost (wx_to_std (d->GetPath ()), "r");
 		if (!f) {
-			throw OpenFileError (wx_to_std (d->GetPath ()), errno, false);
+			throw OpenFileError (wx_to_std (d->GetPath ()), errno, OpenFileError::WRITE);
 		}
 
 		string current;
@@ -954,7 +954,7 @@ KeysPage::export_decryption_certificate ()
 		boost::filesystem::path path (wx_to_std(d->GetPath()));
 		FILE* f = fopen_boost (path, "w");
 		if (!f) {
-			throw OpenFileError (path, errno, false);
+			throw OpenFileError (path, errno, OpenFileError::WRITE);
 		}
 
 		string const s = Config::instance()->decryption_chain()->leaf().certificate (true);