Unit tests covering builtin mode

Add unit test for builtin mode logic as well as few tidy-up in the code
(adding some assertions and API documentation), in particular, to make
it clear that builtin-mode is tight to an opened clause, so it must be
set to a currently opened clause.

lib/isc/cfgmgr.c

@@ -391,6 +391,7 @@ isc_cfgmgr_builtinon(void) {
 	REQUIRE(isc__cfgmgr_env != NULL);
 	REQUIRE(isc__cfgmgr_ctx.txn != NULL);
 	REQUIRE(isc__cfgmgr_ctx.cursor != NULL);
+	REQUIRE(ISC_LIST_EMPTY(isc__cfgmgr_ctx.openedclauses) == false);
 	REQUIRE(isc__cfgmgr_ctx.builtinon == false);
 
 	isc__cfgmgr_ctx.builtinon = true;
@@ -406,6 +407,7 @@ isc_cfgmgr_builtinoff(void) {
 	REQUIRE(isc__cfgmgr_ctx.txn != NULL);
 	REQUIRE(isc__cfgmgr_ctx.cursor != NULL);
 	REQUIRE(isc__cfgmgr_ctx.builtinon == true);
+	REQUIRE(ISC_LIST_EMPTY(isc__cfgmgr_ctx.openedclauses) == false);
 	REQUIRE(strncmp(isc__cfgmgr_ctx.prefix, BUILTIN_PREFIX,
 			strlen(BUILTIN_PREFIX)) == 0);
 
@@ -488,6 +490,9 @@ isc_cfgmgr_close(void) {
 	REQUIRE(isc__cfgmgr_ctx.txn != NULL);
 	REQUIRE(isc__cfgmgr_ctx.cursor != NULL);
 
+	if (isc__cfgmgr_ctx.builtinon) {
+		isc_cfgmgr_builtinoff();
+	}
 	clause = ISC_LIST_HEAD(isc__cfgmgr_ctx.openedclauses);
 	ISC_LIST_UNLINK(isc__cfgmgr_ctx.openedclauses, clause, link);
 	isc_mem_free(isc__cfgmgr_mctx, clause->name);

lib/isc/include/isc/cfgmgr.h

@@ -48,7 +48,10 @@ typedef struct isc_cfgmgr_val {
  * Thoe both function enable or disable the "builtin" mode. (disabled by
  * default, and automatically disabled anytime a transaction finished). This
  * implicitely namespacing the clause name  when reading or
- * writting values in cfgmgr. Must be called under a transaction.
+ * writting values in cfgmgr. Must be called under a transaction and with an
+ * opened clause.
+ *
+ * Closing the close automatically turn the builtin mode off.
  */
 void
 isc_cfgmgr_builtinon(void);

tests/isc/cfgmgr_test.c

@@ -1531,6 +1531,94 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_opentests) {
 	isc_cfgmgr_deinit();
 }
 
+ISC_RUN_TEST_IMPL(isc_cfgmgr_builtintests) {
+	isc_result_t result;
+	isc_cfgmgr_val_t val;
+
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	result = isc_cfgmgr_rwtransaction();
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	isc_cfgmgr_newclause("foo");
+
+	/*
+	 * Set a user property and check that it is not possible to read it
+	 * from builtin mode
+	 */
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING,
+				  .string = "userval1" };
+	result = isc_cfgmgr_setval("userprop1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	expect_assert_failure(isc_cfgmgr_builtinoff());
+	isc_cfgmgr_builtinon();
+	expect_assert_failure(isc_cfgmgr_builtinon());
+
+	result = isc_cfgmgr_getval("userprop1", &val);
+	assert_int_equal(result, ISC_R_NOTFOUND);
+
+	/*
+	 * Set a builtin property and check that is is readable from non-builtin
+	 * mode
+	 */
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_UINT32, .uint32 = 1234 };
+	result = isc_cfgmgr_setval("builtinval1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	val = (isc_cfgmgr_val_t){};
+	isc_cfgmgr_builtinoff();
+	expect_assert_failure(isc_cfgmgr_builtinoff());
+
+	result = isc_cfgmgr_getval("builtinval1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	assert_int_equal(val.type, ISC_CFGMGR_UINT32);
+	assert_int_equal(val.uint32, 1234);
+
+	/*
+	 * user override builtin property, now only the user value is visible in
+	 * builtin mode off, but the original value is still readable with
+	 * builtin mode on
+	 */
+	val.uint32 = 5678;
+	result = isc_cfgmgr_setval("builtinval1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	val = (isc_cfgmgr_val_t){};
+	result = isc_cfgmgr_getval("builtinval1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	assert_int_equal(val.type, ISC_CFGMGR_UINT32);
+	assert_int_equal(val.uint32, 5678);
+
+	isc_cfgmgr_builtinon();
+	val = (isc_cfgmgr_val_t){};
+	result = isc_cfgmgr_getval("builtinval1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	assert_int_equal(val.type, ISC_CFGMGR_UINT32);
+	assert_int_equal(val.uint32, 1234);
+
+	/*
+	 * builtin mode on is automatically turned off when closing a clause
+	 * (commit automatically clause opened clauses)
+	 */
+	result = isc_cfgmgr_commit();
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	result = isc_cfgmgr_transaction();
+	result = isc_cfgmgr_open("foo");
+	expect_assert_failure(isc_cfgmgr_builtinoff());
+	isc_cfgmgr_builtinon();
+	isc_cfgmgr_close();
+
+	result = isc_cfgmgr_open("foo");
+	expect_assert_failure(isc_cfgmgr_builtinoff());
+	isc_cfgmgr_builtinon();
+
+	isc_cfgmgr_rollback();
+	isc_cfgmgr_deinit();
+}
+
 ISC_TEST_LIST_START
 ISC_TEST_ENTRY(isc_cfgmgr_assertions)
 ISC_TEST_ENTRY(isc_cfgmgr_rw)
@@ -1546,5 +1634,6 @@ ISC_TEST_ENTRY(isc_cfgmgr_parseid)
 ISC_TEST_ENTRY(isc_cfgmgr_validation_error)
 ISC_TEST_ENTRY(isc_cfgmgr_getparent)
 ISC_TEST_ENTRY(isc_cfgmgr_opentests)
+ISC_TEST_ENTRY(isc_cfgmgr_builtintests)
 ISC_TEST_LIST_END
 ISC_TEST_MAIN