throw nv_error_state when calling add_* on an nvlist in error state

5 files changed, 170 insertions, 8 deletions

diff --git a/libnvxx/nv_list.cc b/libnvxx/nv_list.cc
index d14d3da..69252ab 100644
--- a/libnvxx/nv_list.cc
+++ b/libnvxx/nv_list.cc
@@ -182,6 +182,8 @@ __nv_list::free(std::string_view key)
 void
 __nv_list::add_null(std::string_view key)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	::nvlist_add_null(__m_nv, skey.c_str());
@@ -212,6 +214,8 @@ __nv_list::free_null(std::string_view key)
 void
 __nv_list::add_bool(std::string_view key, bool value)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	::nvlist_add_bool(__m_nv, skey.c_str(), value);
@@ -260,6 +264,8 @@ void
 __nv_list::add_bool_array(std::string_view key,
 			  std::span<bool const> value)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	::nvlist_add_bool_array(__m_nv, skey.c_str(),
@@ -306,6 +312,8 @@ __nv_list::free_bool_array(std::string_view key)
 void
 __nv_list::add_number(std::string_view key, std::uint64_t value)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	::nvlist_add_number(__m_nv, skey.c_str(), value);
@@ -355,6 +363,8 @@ void
 __nv_list::add_number_array(std::string_view key,
 			    std::span<std::uint64_t const> value)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	::nvlist_add_number_array(__m_nv, skey.c_str(),
@@ -402,6 +412,8 @@ __nv_list::free_number_array(std::string_view key)
 void
 __nv_list::add_string(std::string_view key, std::string_view value)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	::nvlist_add_string(__m_nv, skey.c_str(), std::string(value).c_str());
@@ -446,6 +458,8 @@ void
 __nv_list::add_string_array(std::string_view key,
 			    std::span<std::string_view const> value)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	// nvlist_add_string_array expects an array of NUL-terminated
@@ -516,6 +530,8 @@ __nv_list::free_string_array(std::string_view key)
 void
 __nv_list::add_nvlist(std::string_view key, const_nv_list const &other)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	::nvlist_add_nvlist(__m_nv, skey.c_str(), other.__m_nv);
@@ -568,6 +584,8 @@ void
 __nv_list::add_nvlist_array(std::string_view key,
 			    std::span<const_nv_list const> value)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	auto ptrs = value
@@ -594,6 +612,8 @@ void
 __nv_list::add_nvlist_array(std::string_view key,
 			    std::span<nv_list const> value)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	auto ptrs = value
@@ -664,6 +684,8 @@ __nv_list::take_descriptor(std::string_view key)
 void
 __nv_list::add_descriptor(std::string_view key, int value)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	::nvlist_add_descriptor(__m_nv, skey.c_str(), value);
@@ -705,6 +727,8 @@ void
 __nv_list::add_descriptor_array(std::string_view key,
 				std::span<int const> value)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	::nvlist_add_descriptor_array(__m_nv, skey.c_str(),
@@ -756,6 +780,8 @@ __nv_list::take_descriptor_array(std::string_view key)
 void
 __nv_list::add_binary(std::string_view key, std::span<std::byte const> value)
 {
+	__throw_if_error();
+
 	auto skey = std::string(key);
 
 	::nvlist_add_binary(__m_nv, skey.c_str(),
diff --git a/libnvxx/nvxx.3 b/libnvxx/nvxx.3
index b1c0223..1ea9967 100644
--- a/libnvxx/nvxx.3
+++ b/libnvxx/nvxx.3
@@ -439,6 +439,9 @@ If a value of the given name is already present in the nvlist, and the nvlist
 does not permit duplicate value names, then an exception of type
 .Vt nv_key_exists
 is thrown, and the nvlist is placed in the error state.
+If the nvlist is in an error state, then an exception of type
+.Vt nv_error_state
+is thrown, and the key is not added.
 .Pp
 The
 .Fn add_descriptor
diff --git a/libnvxx/nvxx.cc b/libnvxx/nvxx.cc
index 0932f9a..aad95bd 100644
--- a/libnvxx/nvxx.cc
+++ b/libnvxx/nvxx.cc
@@ -63,14 +63,11 @@ __nv_list_base::__free_nv() noexcept
 }
 
 void
-__nv_list_base::__throw_if_error()
+__nv_list_base::__throw_if_error() const
 {
-	auto err = ::nvlist_error(__m_nv);
-
-	if (err == 0)
-		return;
-
-	throw nv_error_state(std::error_code(err, std::generic_category()));
+	if (auto err = ::nvlist_error(__m_nv); err != 0)
+		throw nv_error_state(std::error_code(err,
+						     std::generic_category()));
 }
 
 } // namespace bsd::__detail
diff --git a/libnvxx/nvxx_base.h b/libnvxx/nvxx_base.h
index 6dc3ca5..cad6817 100644
--- a/libnvxx/nvxx_base.h
+++ b/libnvxx/nvxx_base.h
@@ -109,7 +109,7 @@ protected:
 	~__nv_list_base();
 	void __free_nv() noexcept;
 
-	void __throw_if_error();
+	void __throw_if_error() const;
 
 	::nvlist_t *__m_nv{};
 	__nvlist_owning __m_owning;
diff --git a/libnvxx/tests/nvxx_basic.cc b/libnvxx/tests/nvxx_basic.cc
index ded7d48..fa17397 100644
--- a/libnvxx/tests/nvxx_basic.cc
+++ b/libnvxx/tests/nvxx_basic.cc
@@ -93,6 +93,16 @@ TEST_CASE(nvxx_add_null)
 	ATF_REQUIRE_EQ(true, nvl.exists(key));
 }
 
+TEST_CASE(nvxx_add_null_error)
+{
+	using namespace std::literals;
+	auto constexpr key = "test_null"sv;
+
+	auto nvl = bsd::nv_list();
+	nvl.set_error(EINVAL);
+	ATF_REQUIRE_THROW(bsd::nv_error_state, nvl.add_null(key));
+}
+
 TEST_CASE(nvxx_add_duplicate_null)
 {
 	using namespace std::literals;
@@ -138,6 +148,17 @@ TEST_CASE(nvxx_add_bool)
 	ATF_REQUIRE_EQ(value, nvl.get_bool(key));
 }
 
+TEST_CASE(nvxx_add_bool_error)
+{
+	using namespace std::literals;
+	auto constexpr key = "test_bool"sv;
+	auto constexpr value = true;
+
+	auto nvl = bsd::nv_list();
+	nvl.set_error(EINVAL);
+	ATF_REQUIRE_THROW(bsd::nv_error_state, nvl.add_bool(key, value));
+}
+
 TEST_CASE(nvxx_add_duplicate_bool)
 {
 	using namespace std::literals;
@@ -214,6 +235,17 @@ TEST_CASE(nvxx_add_bool_array)
 	ATF_REQUIRE_EQ(false, nvl.exists(key));
 }
 
+TEST_CASE(nvxx_add_bool_array_error)
+{
+	using namespace std::literals;
+	auto constexpr key = "test_bool"sv;
+	auto value = std::vector<bool>{true, false};
+
+	auto nvl = bsd::nv_list();
+	nvl.set_error(EINVAL);
+	ATF_REQUIRE_THROW(bsd::nv_error_state, nvl.add_bool_range(key, value));
+}
+
 TEST_CASE(nvxx_get_nonexistent_bool_array)
 {
 	auto nvl = bsd::nv_list();
@@ -299,6 +331,18 @@ TEST_CASE(nvxx_add_number)
 	ATF_REQUIRE_EQ(value, nvl.get_number(key));
 }
 
+TEST_CASE(nvxx_add_number_error)
+{
+	using namespace std::literals;
+	auto constexpr key = "test_number"sv;
+	auto constexpr value = 42_u64;
+
+	auto nvl = bsd::nv_list();
+	nvl.set_error(EINVAL);
+
+	ATF_REQUIRE_THROW(bsd::nv_error_state, nvl.add_number(key, value));
+}
+
 TEST_CASE(nvxx_add_duplicate_number)
 {
 	using namespace std::literals;
@@ -374,6 +418,18 @@ TEST_CASE(nvxx_add_number_array)
 	ATF_REQUIRE_EQ(false, nvl.exists(key));
 }
 
+TEST_CASE(nvxx_add_number_array_error)
+{
+	using namespace std::literals;
+	auto constexpr key = "test_number"sv;
+	auto value = std::vector<std::uint64_t>{42, 666};
+
+	auto nvl = bsd::nv_list();
+	nvl.set_error(EINVAL);
+	ATF_REQUIRE_THROW(bsd::nv_error_state,
+			  nvl.add_number_range(key, value));
+}
+
 TEST_CASE(nvxx_get_nonexistent_number_array)
 {
 	auto nvl = bsd::nv_list();
@@ -524,6 +580,29 @@ TEST_CASE(nvxx_add_string_array)
 	ATF_REQUIRE_EQ(false, nvl.exists(key));
 }
 
+TEST_CASE(nvxx_add_string_error)
+{
+	using namespace std::literals;
+	auto constexpr key = "test_string"sv;
+	auto constexpr value = "test"sv;
+
+	auto nvl = bsd::nv_list{};
+	nvl.set_error(EINVAL);
+	ATF_REQUIRE_THROW(bsd::nv_error_state, nvl.add_string(key, value));
+}
+
+TEST_CASE(nvxx_add_string_array_error)
+{
+	using namespace std::literals;
+	auto constexpr key = "test_string"sv;
+	auto values = std::vector{"one"sv, "two"sv, "three"sv};
+
+	auto nvl = bsd::nv_list();
+	nvl.set_error(EINVAL);
+	ATF_REQUIRE_THROW(bsd::nv_error_state,
+			  nvl.add_string_range(key, values));
+}
+
 TEST_CASE(nvxx_get_nonexistent_string_array)
 {
 	auto nvl = bsd::nv_list();
@@ -606,6 +685,17 @@ TEST_CASE(nvxx_add_nvlist)
 		       		.get_number("test_number"));
 }
 
+TEST_CASE(nvxx_add_nvlist_error)
+{
+	using namespace std::literals;
+	auto key = "test_nvlist"sv;
+	auto value = bsd::nv_list{};
+
+	auto nvl = bsd::nv_list{};
+	nvl.set_error(EINVAL);
+	ATF_REQUIRE_THROW(bsd::nv_error_state, nvl.add_nvlist(key, value));
+}
+
 TEST_CASE(nvxx_add_duplicate_nvlist)
 {
 	using namespace std::literals;
@@ -690,6 +780,18 @@ TEST_CASE(nvxx_add_nvlist_array)
 	ATF_REQUIRE_EQ(n2, 2);
 }
 
+TEST_CASE(nvxx_add_nvlist_array_error)
+{
+	using namespace std::literals;
+	auto constexpr key = "nvls"sv;
+	auto value = std::vector{bsd::nv_list{}, bsd::nv_list{}};
+
+	auto nvl = bsd::nv_list();
+	nvl.set_error(EINVAL);
+	ATF_REQUIRE_THROW(bsd::nv_error_state,
+			  nvl.add_nvlist_array(key, value));
+}
+
 TEST_CASE(nvxx_get_nonexistent_nvlist_array)
 {
 	auto nvl = bsd::nv_list();
@@ -748,6 +850,17 @@ TEST_CASE(nvxx_add_descriptor)
 	ATF_REQUIRE_EQ('4', buf[3]);
 }
 
+TEST_CASE(nvxx_add_descriptor_error)
+{
+	using namespace std::literals;
+	auto key = "test_descriptor"sv;
+
+	auto nvl = bsd::nv_list{};
+	nvl.set_error(EINVAL);
+	ATF_REQUIRE_THROW(bsd::nv_error_state,
+			  nvl.add_descriptor(key, 0));
+}
+
 TEST_CASE(nvxx_get_nonexistent_descriptor)
 {
 	auto nvl = bsd::nv_list();
@@ -800,6 +913,18 @@ TEST_CASE(nvxx_add_binary)
 	ATF_REQUIRE_EQ(true, std::ranges::equal(data, data2));
 }
 
+TEST_CASE(nvxx_add_binary_error)
+{
+	using namespace std::literals;
+	auto constexpr key = "test_binary"sv;
+	auto value = std::array<std::byte, 16>{};
+
+	auto nvl = bsd::nv_list();
+	nvl.set_error(EINVAL);
+	ATF_REQUIRE_THROW(bsd::nv_error_state,
+			  nvl.add_binary("test_binary", value));
+}
+
 TEST_CASE(nvxx_add_duplicate_binary)
 {
 	using namespace std::literals;
@@ -869,60 +994,71 @@ ATF_INIT_TEST_CASES(tcs)
 	ATF_ADD_TEST_CASE(tcs, nvxx_ignore_case);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_null);
+	ATF_ADD_TEST_CASE(tcs, nvxx_add_null_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_duplicate_null);
 	ATF_ADD_TEST_CASE(tcs, nvxx_free_null);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_bool);
+	ATF_ADD_TEST_CASE(tcs, nvxx_add_bool_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_duplicate_bool);
 	ATF_ADD_TEST_CASE(tcs, nvxx_get_nonexistent_bool);
 	ATF_ADD_TEST_CASE(tcs, nvxx_take_bool);
 	ATF_ADD_TEST_CASE(tcs, nvxx_free_bool);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_bool_array);
+	ATF_ADD_TEST_CASE(tcs, nvxx_add_bool_array_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_duplicate_bool_array);
 	ATF_ADD_TEST_CASE(tcs, nvxx_get_nonexistent_bool_array);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_bool_range);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_bool_contig_range);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_number);
+	ATF_ADD_TEST_CASE(tcs, nvxx_add_number_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_duplicate_number);
 	ATF_ADD_TEST_CASE(tcs, nvxx_get_nonexistent_number);
 	ATF_ADD_TEST_CASE(tcs, nvxx_take_number);
 	ATF_ADD_TEST_CASE(tcs, nvxx_free_number);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_number_array);
+	ATF_ADD_TEST_CASE(tcs, nvxx_add_number_array_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_duplicate_number_array);
 	ATF_ADD_TEST_CASE(tcs, nvxx_get_nonexistent_number_array);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_number_range);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_number_contig_range);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_string);
+	ATF_ADD_TEST_CASE(tcs, nvxx_add_string_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_duplicate_string);
 	ATF_ADD_TEST_CASE(tcs, nvxx_get_nonexistent_string);
 	ATF_ADD_TEST_CASE(tcs, nvxx_take_string);
 	ATF_ADD_TEST_CASE(tcs, nvxx_free_string);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_string_array);
+	ATF_ADD_TEST_CASE(tcs, nvxx_add_string_array_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_duplicate_string_array);
 	ATF_ADD_TEST_CASE(tcs, nvxx_get_nonexistent_string_array);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_string_range);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_string_contig_range);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_nvlist);
+	ATF_ADD_TEST_CASE(tcs, nvxx_add_nvlist_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_duplicate_nvlist);
 	ATF_ADD_TEST_CASE(tcs, nvxx_get_nonexistent_nvlist);
 	ATF_ADD_TEST_CASE(tcs, nvxx_take_nvlist);
 	ATF_ADD_TEST_CASE(tcs, nvxx_free_nvlist);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_nvlist_array);
+	ATF_ADD_TEST_CASE(tcs, nvxx_add_nvlist_array_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_duplicate_nvlist_array);
 	ATF_ADD_TEST_CASE(tcs, nvxx_get_nonexistent_nvlist_array);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_descriptor);
+	ATF_ADD_TEST_CASE(tcs, nvxx_add_descriptor_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_duplicate_descriptor);
 	ATF_ADD_TEST_CASE(tcs, nvxx_get_nonexistent_descriptor);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_binary);
+	ATF_ADD_TEST_CASE(tcs, nvxx_add_binary_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_duplicate_binary);
 	ATF_ADD_TEST_CASE(tcs, nvxx_get_nonexistent_binary);
 	ATF_ADD_TEST_CASE(tcs, nvxx_take_binary);