From b57d40947e55a7ae34e2924abca21dd45acb6ace Mon Sep 17 00:00:00 2001
From: Daniel Salzman <daniel.salzman@nic.cz>
Date: Sat, 17 Sep 2016 00:09:14 +0200
Subject: [PATCH] confio: remove confio parameter from conf_io_set

---
 src/knot/conf/confio.c  | 64 ++++++++++++++++----------------------
 src/knot/conf/confio.h  |  4 +--
 src/knot/ctl/commands.c |  7 +----
 tests/confio.c          | 68 ++++++++++++++++++-----------------------
 4 files changed, 58 insertions(+), 85 deletions(-)

diff --git a/src/knot/conf/confio.c b/src/knot/conf/confio.c
index 97fd0eb90e..977a1049f1 100644
--- a/src/knot/conf/confio.c
+++ b/src/knot/conf/confio.c
@@ -872,42 +872,28 @@ static int set_item(
 	}
 
 	// Postpone group callbacks to config check.
-	if (io->key0->type != YP_TGRP) {
-		conf_check_t args = {
-			.conf = conf(),
-			.txn = conf()->io.txn,
-			.item = (io->key1 != NULL) ? io->key1 : io->key0,
-			.id = io->id,
-			.id_len = io->id_len,
-			.data = io->data.bin,
-			.data_len = io->data.bin_len
-		};
-
-		// Call the item callbacks.
-		io->error.code = conf_exec_callbacks(&args);
-		if (io->error.code != KNOT_EOK) {
-			io->error.str = args.err_str;
-			ret = FCN(io);
-			if (ret == KNOT_EOK) {
-				ret = io->error.code;
-			}
-		}
+	if (io->key0->type == YP_TGRP) {
+		return KNOT_EOK;
 	}
 
-	return ret;
+	conf_check_t args = {
+		.conf = conf(),
+		.txn = conf()->io.txn,
+		.item = (io->key1 != NULL) ? io->key1 : io->key0,
+		.data = io->data.bin,
+		.data_len = io->data.bin_len
+	};
+
+	// Call the item callbacks (include).
+	return conf_exec_callbacks(&args);
 }
 
 int conf_io_set(
 	const char *key0,
 	const char *key1,
 	const char *id,
-	const char *data,
-	conf_io_t *io)
+	const char *data)
 {
-	if (io == NULL) {
-		return KNOT_EINVAL;
-	}
-
 	assert(conf() != NULL);
 
 	if (conf()->io.txn == NULL) {
@@ -936,11 +922,13 @@ int conf_io_set(
 	yp_flag_t upd_flags = node->item->flags;
 	conf_io_type_t upd_type = CONF_IO_TNONE;
 
+	conf_io_t io = { NULL };
+
 	// Key1 is not a group identifier.
 	if (parent != NULL) {
 		upd_type = CONF_IO_TCHANGE;
 		upd_flags |= parent->item->flags;
-		io_reset_bin(io, parent->item, node->item, parent->id,
+		io_reset_bin(&io, parent->item, node->item, parent->id,
 		             parent->id_len, node->data, node->data_len);
 	// No key1 but a group identifier.
 	} else if (node->id_len != 0) {
@@ -950,11 +938,11 @@ int conf_io_set(
 
 		upd_type = CONF_IO_TSET;
 		upd_flags |= node->item->var.g.id->flags;
-		io_reset_bin(io, node->item, node->item->var.g.id, node->id,
+		io_reset_bin(&io, node->item, node->item->var.g.id, node->id,
 		             node->id_len, NULL, 0);
 	// Ensure some data for non-group items (include).
 	} else if (node->item->type == YP_TGRP || node->data_len != 0) {
-		io_reset_bin(io, node->item, NULL, node->id, node->id_len,
+		io_reset_bin(&io, node->item, NULL, node->id, node->id_len,
 		             node->data, node->data_len);
 	// Non-group without data.
 	} else {
@@ -963,10 +951,10 @@ int conf_io_set(
 	}
 
 	// Set the item for all identifiers by default.
-	if (io->key0->type == YP_TGRP && io->key1 != NULL &&
-	    (io->key0->flags & YP_FMULTI) != 0 && io->id_len == 0) {
+	if (io.key0->type == YP_TGRP && io.key1 != NULL &&
+	    (io.key0->flags & YP_FMULTI) != 0 && io.id_len == 0) {
 		conf_iter_t iter;
-		ret = conf_db_iter_begin(conf(), conf()->io.txn, io->key0->name,
+		ret = conf_db_iter_begin(conf(), conf()->io.txn, io.key0->name,
 		                         &iter);
 		switch (ret) {
 		case KNOT_EOK:
@@ -980,14 +968,14 @@ int conf_io_set(
 
 		while (ret == KNOT_EOK) {
 			// Get the identifier.
-			ret = conf_db_iter_id(conf(), &iter, &io->id, &io->id_len);
+			ret = conf_db_iter_id(conf(), &iter, &io.id, &io.id_len);
 			if (ret != KNOT_EOK) {
 				conf_db_iter_finish(conf(), &iter);
 				goto set_error;
 			}
 
 			// Set the data.
-			ret = set_item(io);
+			ret = set_item(&io);
 			if (ret != KNOT_EOK) {
 				conf_db_iter_finish(conf(), &iter);
 				goto set_error;
@@ -999,17 +987,17 @@ int conf_io_set(
 			goto set_error;
 		}
 
-		upd_changes(io, upd_type, upd_flags, true);
+		upd_changes(&io, upd_type, upd_flags, true);
 
 		ret = KNOT_EOK;
 		goto set_error;
 	}
 
 	// Set the item with a possible identifier.
-	ret = set_item(io);
+	ret = set_item(&io);
 
 	if (ret == KNOT_EOK) {
-		upd_changes(io, upd_type, upd_flags, false);
+		upd_changes(&io, upd_type, upd_flags, false);
 	}
 set_error:
 	yp_scheme_check_deinit(ctx);
diff --git a/src/knot/conf/confio.h b/src/knot/conf/confio.h
index bb6e0d87b1..cf3a043a15 100644
--- a/src/knot/conf/confio.h
+++ b/src/knot/conf/confio.h
@@ -188,7 +188,6 @@ int conf_io_get(
  * \param[in] key1  Item name (NULL to add identifier only).
  * \param[in] id    Section identifier name (NULL to consider all section identifiers).
  * \param[in] data  Item data to set/add.
- * \param[out] io   Operation output (callback error output).
  *
  * \return Error code, KNOT_EOK if success.
  */
@@ -196,8 +195,7 @@ int conf_io_set(
 	const char *key0,
 	const char *key1,
 	const char *id,
-	const char *data,
-	conf_io_t *io
+	const char *data
 );
 
 /*!
diff --git a/src/knot/ctl/commands.c b/src/knot/ctl/commands.c
index 3dd0f948b9..2483afda4c 100644
--- a/src/knot/ctl/commands.c
+++ b/src/knot/ctl/commands.c
@@ -1188,11 +1188,6 @@ static int ctl_conf_read(ctl_args_t *args, ctl_cmd_t cmd)
 
 static int ctl_conf_modify(ctl_args_t *args, ctl_cmd_t cmd)
 {
-	conf_io_t io = {
-		.fcn = send_block,
-		.misc = args->ctl
-	};
-
 	// Start child transaction.
 	int ret = conf_io_begin(true);
 	if (ret != KNOT_EOK) {
@@ -1208,7 +1203,7 @@ static int ctl_conf_modify(ctl_args_t *args, ctl_cmd_t cmd)
 
 		switch (cmd) {
 		case CTL_CONF_SET:
-			ret = conf_io_set(key0, key1, id, data, &io);
+			ret = conf_io_set(key0, key1, id, data);
 			break;
 		case CTL_CONF_UNSET:
 			ret = conf_io_unset(key0, key1, id, data);
diff --git a/tests/confio.c b/tests/confio.c
index 7df2f61ba8..175765e6a3 100644
--- a/tests/confio.c
+++ b/tests/confio.c
@@ -207,19 +207,17 @@ static void test_conf_io_abort(void)
 #if defined(__OpenBSD__)
 	SKIP_OPENBSD
 #else
-	conf_io_t io = { NULL };
-
 	// Test child persistence after subchild abort.
 
 	ok(conf_io_begin(false) == KNOT_EOK, "begin parent txn");
 	char idx[2] = { '0' };
-	ok(conf_io_set("server", "version", NULL, idx, &io) ==
+	ok(conf_io_set("server", "version", NULL, idx) ==
 	   KNOT_EOK, "set single value '%s'", idx);
 
 	for (int i = 1; i < CONF_MAX_TXN_DEPTH; i++) {
 		char idx[2] = { '0' + i };
 		ok(conf_io_begin(true) == KNOT_EOK, "begin child txn %s", idx);
-		ok(conf_io_set("server", "version", NULL, idx, &io) ==
+		ok(conf_io_set("server", "version", NULL, idx) ==
 		   KNOT_EOK, "set single value '%s'", idx);
 	}
 
@@ -239,7 +237,7 @@ static void test_conf_io_abort(void)
 	ok(conf_io_begin(false) == KNOT_EOK, "begin new parent txn");
 	ok(conf_io_begin(true) == KNOT_EOK, "begin child txn");
 	ok(conf_io_begin(true) == KNOT_EOK, "begin subchild txn");
-	ok(conf_io_set("server", "version", NULL, "text", &io) ==
+	ok(conf_io_set("server", "version", NULL, "text") ==
 	   KNOT_EOK, "set single value");
 	ok(conf_io_commit(true) == KNOT_EOK, "commit subchild txn");
 	conf_val_t val = conf_get_txn(conf(), conf()->io.txn, C_SERVER, C_VERSION);
@@ -265,19 +263,17 @@ static void test_conf_io_commit(void)
 #if defined(__OpenBSD__)
 	SKIP_OPENBSD
 #else
-	conf_io_t io = { NULL };
-
 	// Test subchild persistence after commit.
 
 	ok(conf_io_begin(false) == KNOT_EOK, "begin parent txn");
 	char idx[2] = { '0' };
-	ok(conf_io_set("server", "version", NULL, idx, &io) ==
+	ok(conf_io_set("server", "version", NULL, idx) ==
 	   KNOT_EOK, "set single value '%s'", idx);
 
 	for (int i = 1; i < CONF_MAX_TXN_DEPTH; i++) {
 		char idx[2] = { '0' + i };
 		ok(conf_io_begin(true) == KNOT_EOK, "begin child txn %s", idx);
-		ok(conf_io_set("server", "version", NULL, idx, &io) ==
+		ok(conf_io_set("server", "version", NULL, idx) ==
 		   KNOT_EOK, "set single value '%s'", idx);
 	}
 
@@ -319,22 +315,22 @@ static void test_conf_io_check(void)
 	ok(conf_io_begin(false) == KNOT_EOK, "begin txn");
 
 	// Section check.
-	ok(conf_io_set("remote", "id", NULL, "remote1", &io) ==
+	ok(conf_io_set("remote", "id", NULL, "remote1") ==
 	   KNOT_EOK, "set remote id");
 	ok(conf_io_check(&io) ==
 	   KNOT_EINVAL, "check missing remote address");
 	ok(io.error.code == KNOT_EINVAL, "compare error code");
 
-	ok(conf_io_set("remote", "address", "remote1", "1.1.1.1", &io) ==
+	ok(conf_io_set("remote", "address", "remote1", "1.1.1.1") ==
 	   KNOT_EOK, "set remote address");
 	ok(conf_io_check(&io) ==
 	   KNOT_EOK, "check remote address");
 	ok(io.error.code == KNOT_EOK, "compare error code");
 
 	// Item check.
-	ok(conf_io_set("zone", "domain", NULL, ZONE1, &io) ==
+	ok(conf_io_set("zone", "domain", NULL, ZONE1) ==
 	   KNOT_EOK, "set zone domain "ZONE1);
-	ok(conf_io_set("zone", "master", ZONE1, "remote1", &io) ==
+	ok(conf_io_set("zone", "master", ZONE1, "remote1") ==
 	   KNOT_EOK, "set zone master");
 
 	ok(conf_io_check(&io) ==
@@ -352,45 +348,41 @@ static void test_conf_io_check(void)
 
 static void test_conf_io_set(void)
 {
-	conf_io_t io = { NULL };
-
 	// ERR no txn.
-	ok(conf_io_set("server", "version", NULL, "text", &io) ==
+	ok(conf_io_set("server", "version", NULL, "text") ==
 	   KNOT_TXN_ENOTEXISTS, "set without active txn");
 
 	ok(conf_io_begin(false) == KNOT_EOK, "begin txn");
 
 	// ERR.
-	ok(conf_io_set(NULL, NULL, NULL, NULL, &io) ==
+	ok(conf_io_set(NULL, NULL, NULL, NULL) ==
 	   KNOT_EINVAL, "set NULL key0");
-	ok(conf_io_set("", NULL, NULL, NULL, &io) ==
+	ok(conf_io_set("", NULL, NULL, NULL) ==
 	   KNOT_YP_EINVAL_ITEM, "set empty key0");
-	ok(conf_io_set("uknown", NULL, NULL, NULL, &io) ==
+	ok(conf_io_set("uknown", NULL, NULL, NULL) ==
 	   KNOT_YP_EINVAL_ITEM, "set unknown key0");
-	ok(conf_io_set("server", "unknown", NULL, NULL, &io) ==
+	ok(conf_io_set("server", "unknown", NULL, NULL) ==
 	   KNOT_YP_EINVAL_ITEM, "set unknown key1");
-	ok(conf_io_set("include", NULL, NULL, NULL, &io) ==
+	ok(conf_io_set("include", NULL, NULL, NULL) ==
 	   KNOT_YP_ENODATA, "set non-group without data");
-	ok(conf_io_set("server", "rate-limit", NULL, "x", &io) ==
+	ok(conf_io_set("server", "rate-limit", NULL, "x") ==
 	   KNOT_EINVAL, "set invalid data");
 
 	// ERR callback
-	ok(io.error.code == KNOT_EOK, "io error check before");
-	ok(conf_io_set("include", NULL, NULL, "invalid", &io) ==
+	ok(conf_io_set("include", NULL, NULL, "invalid") ==
 	   KNOT_EFILE, "set invalid callback value");
-	ok(io.error.code == KNOT_EFILE, "io error check after");
 
 	// Single group, single value.
-	ok(conf_io_set("server", "version", NULL, "text", &io) ==
+	ok(conf_io_set("server", "version", NULL, "text") ==
 	   KNOT_EOK, "set single value");
 	conf_val_t val = conf_get_txn(conf(), conf()->io.txn, C_SERVER, C_VERSION);
 	ok(val.code == KNOT_EOK, "check entry");
 	ok(strcmp(conf_str(&val), "text") == 0, "check entry value");
 
 	// Single group, multi value.
-	ok(conf_io_set("server", "listen", NULL, "1.1.1.1", &io) ==
+	ok(conf_io_set("server", "listen", NULL, "1.1.1.1") ==
 	   KNOT_EOK, "set multivalue 1");
-	ok(conf_io_set("server", "listen", NULL, "1.1.1.2", &io) ==
+	ok(conf_io_set("server", "listen", NULL, "1.1.1.2") ==
 	   KNOT_EOK, "set multivalue 2");
 	val = conf_get_txn(conf(), conf()->io.txn, C_SERVER, C_LISTEN);
 	ok(val.code == KNOT_EOK, "check entry");
@@ -405,24 +397,24 @@ static void test_conf_io_set(void)
 	ok(zone3 != NULL, "create dname "ZONE3);
 
 	// Multi group ids.
-	ok(conf_io_set("zone", "domain", NULL, ZONE1, &io) ==
+	ok(conf_io_set("zone", "domain", NULL, ZONE1) ==
 	   KNOT_EOK, "set zone domain "ZONE1);
-	ok(conf_io_set("zone", NULL, ZONE2, NULL, &io) ==
+	ok(conf_io_set("zone", NULL, ZONE2, NULL) ==
 	   KNOT_EOK, "set zone domain "ZONE2);
 
 	// Multi group, single value.
-	ok(conf_io_set("zone", "file", ZONE1, "name", &io) ==
+	ok(conf_io_set("zone", "file", ZONE1, "name") ==
 	   KNOT_EOK, "set zone file");
 	val = conf_zone_get_txn(conf(), conf()->io.txn, C_FILE, zone1);
 	ok(val.code == KNOT_EOK, "check entry");
 	ok(strcmp(conf_str(&val), "name") == 0, "check entry value");
 
 	// Multi group, single value, bad id.
-	ok(conf_io_set("zone", "file", ZONE3, "name", &io) ==
+	ok(conf_io_set("zone", "file", ZONE3, "name") ==
 	   KNOT_YP_EINVAL_ID, "set zone file");
 
 	// Multi group, single value, all ids.
-	ok(conf_io_set("zone", "comment", NULL, "abc", &io) ==
+	ok(conf_io_set("zone", "comment", NULL, "abc") ==
 	   KNOT_EOK, "set zones comment");
 	val = conf_zone_get_txn(conf(), conf()->io.txn, C_COMMENT, zone1);
 	ok(val.code == KNOT_EOK, "check entry");
@@ -432,9 +424,9 @@ static void test_conf_io_set(void)
 	ok(strcmp(conf_str(&val), "abc") == 0, "check entry value");
 
 	// Prepare different comment.
-	ok(conf_io_set("zone", "domain", NULL, ZONE3, &io) ==
+	ok(conf_io_set("zone", "domain", NULL, ZONE3) ==
 	   KNOT_EOK, "set zone domain "ZONE3);
-	ok(conf_io_set("zone", "comment", ZONE3, "xyz", &io) ==
+	ok(conf_io_set("zone", "comment", ZONE3, "xyz") ==
 	   KNOT_EOK, "set zone comment");
 	val = conf_zone_get_txn(conf(), conf()->io.txn, C_COMMENT, zone3);
 	ok(val.code == KNOT_EOK, "check entry");
@@ -667,7 +659,7 @@ static void test_conf_io_get(void)
 	   KNOT_ENOTSUP, "get non-group item");
 
 	// Update item in the active txn.
-	ok(conf_io_set("server", "version", NULL, "new text", &io) ==
+	ok(conf_io_set("server", "version", NULL, "new text") ==
 	   KNOT_EOK, "set single value");
 
 	// Get new, active txn.
@@ -793,7 +785,7 @@ static void test_conf_io_diff(void)
 	ok(strcmp(ref, out) == 0, "compare result");
 
 	// Update singlevalued item.
-	ok(conf_io_set("server", "version", NULL, "new text", &io) ==
+	ok(conf_io_set("server", "version", NULL, "new text") ==
 	   KNOT_EOK, "set single value");
 
 	*out = '\0';
@@ -805,7 +797,7 @@ static void test_conf_io_diff(void)
 	// Update multivalued item.
 	ok(conf_io_unset("server", "listen", NULL, "1.1.1.1") ==
 	   KNOT_EOK, "unset multivalue");
-	ok(conf_io_set("server", "listen", NULL, "1.1.1.3", &io) ==
+	ok(conf_io_set("server", "listen", NULL, "1.1.1.3") ==
 	   KNOT_EOK, "set multivalue");
 
 	*out = '\0';
-- 
GitLab