construct from an error-state nvlist_t* should be an error

6 files changed, 75 insertions, 14 deletions

diff --git a/libnvxx/const_nv_list.cc b/libnvxx/const_nv_list.cc
index 1ca63d8..19d139e 100644
--- a/libnvxx/const_nv_list.cc
+++ b/libnvxx/const_nv_list.cc
@@ -37,7 +37,7 @@ const_nv_list::const_nv_list() noexcept
 }
 
 // const_cast is safe here since a non-owning nvlist is never modified.
-const_nv_list::const_nv_list(::nvlist_t const *nvl) noexcept
+const_nv_list::const_nv_list(::nvlist_t const *nvl)
 	: __nv_list_base(const_cast<::nvlist_t *>(nvl),
 			 __detail::__nvlist_owning::__non_owning)
 {
diff --git a/libnvxx/nv_list.cc b/libnvxx/nv_list.cc
index 12c2c39..d14d3da 100644
--- a/libnvxx/nv_list.cc
+++ b/libnvxx/nv_list.cc
@@ -39,7 +39,7 @@ nv_list::nv_list(int flags)
 			std::error_code(errno, std::system_category()));
 }
 
-nv_list::nv_list(::nvlist_t *nvl) noexcept
+nv_list::nv_list(::nvlist_t *nvl)
 	: __nv_list_base(nvl, __detail::__nvlist_owning::__owning)
 {
 }
diff --git a/libnvxx/nvxx.cc b/libnvxx/nvxx.cc
index d730f01..0932f9a 100644
--- a/libnvxx/nvxx.cc
+++ b/libnvxx/nvxx.cc
@@ -37,14 +37,16 @@ __nv_list_base::__nv_list_base(int flags)
 {
 	if (__m_nv == nullptr)
 		throw std::system_error(
-			std::error_code(errno, std::system_category()));
+			std::error_code(errno, std::generic_category()));
 }
 
 __nv_list_base::__nv_list_base(nvlist_t *nv, __nvlist_owning owning)
 	: __m_nv(nv)
-	, __m_owning(owning)
+	, __m_owning(__nvlist_owning::__non_owning)
 {
 	assert(nv);
+	__throw_if_error();
+	__m_owning = owning;
 }
 
 __nv_list_base::~__nv_list_base()
@@ -60,4 +62,15 @@ __nv_list_base::__free_nv() noexcept
 		::nvlist_destroy(__m_nv);
 }
 
+void
+__nv_list_base::__throw_if_error()
+{
+	auto err = ::nvlist_error(__m_nv);
+
+	if (err == 0)
+		return;
+
+	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 77e5d53..6dc3ca5 100644
--- a/libnvxx/nvxx_base.h
+++ b/libnvxx/nvxx_base.h
@@ -32,6 +32,9 @@ namespace bsd {
 struct nv_list;
 struct const_nv_list;
 
+/*
+ * Generic base error type.
+ */
 struct nv_error : std::runtime_error {
 	template<typename... _Args>
 	nv_error(std::format_string<_Args...> __fmt, _Args && ...__args)
@@ -41,6 +44,25 @@ struct nv_error : std::runtime_error {
 	}
 };
 
+/*
+ * An operation was attempted on an nv_list whose underlying nvlist_t is in an
+ * error state.  This is a logic error since such operations are documented as
+ * not being possible.
+ */
+struct nv_error_state : nv_error {
+	nv_error_state(std::error_code __error)
+		: nv_error("operation attempted on an "
+			   "nvlist_t in an error state")
+		, error(__error)
+	{
+	}
+
+	std::error_code error;
+};
+
+/*
+ * A get-like function did not find the requested key.
+ */
 struct nv_key_not_found : nv_error {
 	std::string key;
 
@@ -51,6 +73,9 @@ struct nv_key_not_found : nv_error {
 	}
 };
 
+/*
+ * An add-like function found a duplicate key.
+ */
 struct nv_key_exists : nv_error {
 	std::string key;
 
@@ -84,6 +109,8 @@ protected:
 	~__nv_list_base();
 	void __free_nv() noexcept;
 
+	void __throw_if_error();
+
 	::nvlist_t *__m_nv{};
 	__nvlist_owning __m_owning;
 };
@@ -502,7 +529,7 @@ struct const_nv_list final
 	 * destruction.  If the nvlist_t is null, the const_nv_list will be
 	 * empty.
 	 */
-	explicit const_nv_list(::nvlist_t const *) noexcept;
+	explicit const_nv_list(::nvlist_t const *);
 
 	/*
 	 * Copy the nvlist pointer from an existing const_nv_list.  This does
@@ -547,7 +574,7 @@ struct nv_list final
 	/*
 	 * Create an nv_list object that refers to an existing nvlist_t.
 	 */
-	explicit nv_list(::nvlist_t *) noexcept;
+	explicit nv_list(::nvlist_t *);
 
 	/*
 	 * Create an nv_list object by copying an existing const_nv_list object
diff --git a/libnvxx/tests/Makefile b/libnvxx/tests/Makefile
index af44f03..7559989 100644
--- a/libnvxx/tests/Makefile
+++ b/libnvxx/tests/Makefile
@@ -18,13 +18,14 @@
 # ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-PREFIX?=	/usr/local
-TESTSDIR?=	${PREFIX}/tests/nvxx
-ATF_TESTS_CXX=	nvxx_basic nvxx_iterator nvxx_serialize
-CXXSTD=		c++23
+PREFIX?=		/usr/local
+TESTSDIR?=		${PREFIX}/tests/nvxx
+ATF_TESTS_CXX=		nvxx_basic nvxx_iterator nvxx_serialize
+CXXSTD=			c++23
 # Note that we can't use -Werror here because it breaks ATF.
-CXXFLAGS+=	-W -Wall -Wextra
-CFLAGS+=	-I${.CURDIR:H}
-LDFLAGS+=	-lprivateatf-c++ -L${.OBJDIR:H} -lnvxx
+CXXFLAGS+=		-W -Wall -Wextra
+CFLAGS+=		-I${.CURDIR:H}
+LDFLAGS+=		-lprivateatf-c++ -L${.OBJDIR:H} -lnvxx
+LDFLAGS.nvxx_basic+=	-lnv
 
 .include <bsd.test.mk>
diff --git a/libnvxx/tests/nvxx_basic.cc b/libnvxx/tests/nvxx_basic.cc
index 9640d33..ded7d48 100644
--- a/libnvxx/tests/nvxx_basic.cc
+++ b/libnvxx/tests/nvxx_basic.cc
@@ -37,7 +37,7 @@
 	ATF_TEST_CASE_BODY(name)
 
 /*
- * test the default ctor
+ * constructor tests
  */
 
 TEST_CASE(nvxx_ctor_default)
@@ -45,6 +45,24 @@ TEST_CASE(nvxx_ctor_default)
 	auto nvl = bsd::nv_list();
 }
 
+TEST_CASE(nvxx_ctor_error)
+{
+	auto nvp = ::nvlist_create(0);
+	::nvlist_set_error(nvp, EINVAL);
+	try {
+	ATF_REQUIRE_THROW(bsd::nv_error_state,
+			  auto nvl = bsd::nv_list(nvp));
+	} catch (bsd::nv_error_state const &) {}
+}
+
+TEST_CASE(nvxx_const_ctor_error)
+{
+	auto nvp = ::nvlist_create(0);
+	::nvlist_set_error(nvp, EINVAL);
+	ATF_REQUIRE_THROW(bsd::nv_error_state,
+			  auto nvl = bsd::const_nv_list(nvp));
+}
+
 /*
  * test the NV_FLAG_IGNORE_CASE flag.
  */
@@ -846,6 +864,8 @@ TEST_CASE(nvxx_add_binary_range)
 ATF_INIT_TEST_CASES(tcs)
 {
 	ATF_ADD_TEST_CASE(tcs, nvxx_ctor_default);
+	ATF_ADD_TEST_CASE(tcs, nvxx_ctor_error);
+	ATF_ADD_TEST_CASE(tcs, nvxx_const_ctor_error);
 	ATF_ADD_TEST_CASE(tcs, nvxx_ignore_case);
 
 	ATF_ADD_TEST_CASE(tcs, nvxx_add_null);