From 758bcded6f954a60d4b9a38d12b51c68260e08f6 Mon Sep 17 00:00:00 2001
From: Carl Hetherington <cth@carlh.net>
Date: Thu, 8 Sep 2022 22:53:11 +0200
Subject: [PATCH] Fix missing screen description in KDM CLI if you use the
 parameters in the "wrong" order.

---
 src/lib/kdm_cli.cc   | 25 ++++++++++++++-----------
 test/kdm_cli_test.cc |  9 +++++++++
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/src/lib/kdm_cli.cc b/src/lib/kdm_cli.cc
index 4c1a55d7d..56977b521 100644
--- a/src/lib/kdm_cli.cc
+++ b/src/lib/kdm_cli.cc
@@ -422,7 +422,8 @@ try
 	auto filename_format = Config::instance()->kdm_filename_format();
 	optional<string> cinema_name;
 	shared_ptr<Cinema> cinema;
-	string screen_description;
+	optional<boost::filesystem::path> certificate;
+	string screen;
 	list<shared_ptr<Screen>> screens;
 	optional<dcp::EncryptedKDM> dkdm;
 	optional<boost::posix_time::ptime> valid_from;
@@ -534,19 +535,11 @@ try
 			cinema = make_shared<Cinema>(optarg, list<string>(), "", 0, 0);
 			break;
 		case 'S':
-			screen_description = optarg;
+			screen = optarg;
 			break;
 		case 'C':
-		{
-			/* Make a new screen and add it to the current cinema */
-			dcp::CertificateChain chain (dcp::file_to_string(optarg));
-			auto screen = std::make_shared<Screen>(screen_description, "", chain.leaf(), boost::none, vector<TrustedDevice>());
-			if (cinema) {
-				cinema->add_screen (screen);
-			}
-			screens.push_back (screen);
+			certificate = optarg;
 			break;
-		}
 		case 'T':
 			/* A trusted device ends up in the last screen we made */
 			if (!screens.empty ()) {
@@ -562,6 +555,16 @@ try
 		}
 	}
 
+	if (certificate) {
+		/* Make a new screen and add it to the current cinema */
+		dcp::CertificateChain chain(dcp::file_to_string(*certificate));
+		auto screen_to_add = std::make_shared<Screen>(screen, "", chain.leaf(), boost::none, vector<TrustedDevice>());
+		if (cinema) {
+			cinema->add_screen(screen_to_add);
+		}
+		screens.push_back(screen_to_add);
+	}
+
 	if (list_cinemas) {
 		auto cinemas = Config::instance()->cinemas ();
 		for (auto i: cinemas) {
diff --git a/test/kdm_cli_test.cc b/test/kdm_cli_test.cc
index 404f7f8d1..16529a897 100644
--- a/test/kdm_cli_test.cc
+++ b/test/kdm_cli_test.cc
@@ -20,6 +20,7 @@
 
 
 #include "lib/kdm_cli.h"
+#include <boost/filesystem.hpp>
 #include <boost/test/unit_test.hpp>
 #include <iostream>
 
@@ -36,6 +37,8 @@ BOOST_AUTO_TEST_CASE (kdm_cli_test_certificate)
 		"--valid-from", "now",
 		"--valid-duration", "2 weeks",
 		"--certificate", "test/data/cert.pem",
+		"-S", "my great screen",
+		"-o", "build/test",
 		"test/data/dkdm.xml"
 	};
 
@@ -44,12 +47,18 @@ BOOST_AUTO_TEST_CASE (kdm_cli_test_certificate)
 		argv[i] = const_cast<char*>(args[i].c_str());
 	}
 
+	boost::filesystem::path const kdm_filename = "build/test/KDM_Test_FTR-1_F-133_XX-XX_MOS_2K_20220109_SMPTE_OV__my_great_screen.xml";
+	boost::system::error_code ec;
+	boost::filesystem::remove(kdm_filename, ec);
+
 	auto error = kdm_cli (args.size(), argv, [](string s) { std::cout << s << "\n"; });
 	if (error) {
 		std::cout << *error << "\n";
 	}
 	BOOST_CHECK (!error);
 
+	BOOST_CHECK(boost::filesystem::exists(kdm_filename));
+
 	delete[] argv;
 }
 
-- 
2.30.2