[tarantool-patches] [PATCH 2/6] test: remove swim packet filter destructors

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Swim test packet filters are supposed to filter out packets
matching certain criteria or with a probability. They were
implemented as a filter-function and userdata passed into the
former on each invocation. Usually it was allocated on heap and
needed deletion. But it appeared that much simpler is to store
the filters inside struct swim_node, pass it as userdata, and get
rid of userdata destructors and dynamic allocations.

The patch is motivated by necessity to add one new filter, which
anyway will need struct swim_node as userdata.
---
 test/unit/swim_test_transport.c |  14 +---
 test/unit/swim_test_transport.h |  11 +--
 test/unit/swim_test_utils.c     | 127 +++++++++++++++++++++-----------
 3 files changed, 86 insertions(+), 66 deletions(-)

diff --git a/test/unit/swim_test_transport.c b/test/unit/swim_test_transport.c
index d82066388..513274c2c 100644
--- a/test/unit/swim_test_transport.c
+++ b/test/unit/swim_test_transport.c
@@ -109,11 +109,6 @@ swim_test_packet_dup(struct swim_test_packet *p)
 struct swim_fd_filter {
 	/** A function to decide whether to drop a packet. */
 	swim_test_filter_check_f check;
-	/**
-	 * A function called when the filter is deleted to free
-	 * @a udata if necessary.
-	 */
-	swim_test_filter_delete_f delete;
 	/**
 	 * Arbitrary user data. Passed to each call of @a check.
 	 */
@@ -124,14 +119,12 @@ struct swim_fd_filter {
 
 /** Create a new filter. */
 static inline struct swim_fd_filter *
-swim_fd_filter_new(swim_test_filter_check_f check,
-		   swim_test_filter_delete_f delete, void *udata)
+swim_fd_filter_new(swim_test_filter_check_f check, void *udata)
 {
 	struct swim_fd_filter *f = (struct swim_fd_filter *) malloc(sizeof(*f));
 	assert(f != NULL);
 	f->udata = udata;
 	f->check = check;
-	f->delete = delete;
 	rlist_create(&f->in_filters);
 	return f;
 }
@@ -141,7 +134,6 @@ static inline void
 swim_fd_filter_delete(struct swim_fd_filter *filter)
 {
 	rlist_del_entry(filter, in_filters);
-	filter->delete(filter->udata);
 	free(filter);
 }
 
@@ -211,11 +203,11 @@ swim_test_transport_remove_filter(int fd, swim_test_filter_check_f check)
 
 void
 swim_test_transport_add_filter(int fd, swim_test_filter_check_f check,
-			       swim_test_filter_delete_f delete, void *udata)
+			       void *udata)
 {
 	struct swim_fd *sfd = &swim_fd[fd - FAKE_FD_BASE];
 	assert(sfd->is_opened);
-	struct swim_fd_filter *f = swim_fd_filter_new(check, delete, udata);
+	struct swim_fd_filter *f = swim_fd_filter_new(check, udata);
 	swim_test_transport_remove_filter(fd, check);
 	rlist_add_tail_entry(&sfd->filters, f, in_filters);
 }
diff --git a/test/unit/swim_test_transport.h b/test/unit/swim_test_transport.h
index d166dc600..a66cf2d81 100644
--- a/test/unit/swim_test_transport.h
+++ b/test/unit/swim_test_transport.h
@@ -50,13 +50,6 @@ struct ev_loop;
 typedef bool (*swim_test_filter_check_f)(const char *data, int size,
 					 void *udata, int dir);
 
-/**
- * It is possible that a filter is complex and uses helper data
- * allocated somewhere. This function is called when the filter
- * is dropped and allows to free user data.
- */
-typedef void (*swim_test_filter_delete_f)(void *udata);
-
 /**
  * Until there are no new IO events, feed EV_WRITE event to all
  * opened descriptors; EV_READ to ones, who have not empty recv
@@ -84,14 +77,12 @@ swim_test_transport_unblock_fd(int fd);
  * @param fd File descriptor to add filter to.
  * @param check Check function. It is called for each packet and
  *        should return true, when the packet should be dropped.
- * @param delete A destructor for @a udata called when the filter
- *        is dropped.
  * @param udata Arbitrary user data, passed to each @a check
  *        invocation.
  */
 void
 swim_test_transport_add_filter(int fd, swim_test_filter_check_f check,
-			       swim_test_filter_delete_f delete, void *udata);
+			       void *udata);
 
 /**
  * Remove a filter having @a check function. Works just like the
diff --git a/test/unit/swim_test_utils.c b/test/unit/swim_test_utils.c
index 4f488fb7b..0beeab65d 100644
--- a/test/unit/swim_test_utils.c
+++ b/test/unit/swim_test_utils.c
@@ -37,6 +37,49 @@
 #include "fiber.h"
 #include "msgpuck.h"
 
+/**
+ * Drop rate packet filter to drop packets with a certain
+ * probability.
+ */
+struct swim_drop_rate {
+	/** True if should be applied to incoming packets. */
+	bool is_for_in;
+	/** True if should be applied to outgoing packets. */
+	bool is_for_out;
+	/** Drop rate percentage. */
+	double rate;
+};
+
+/** Initialize drop rate packet filter. */
+static inline void
+swim_drop_rate_create(struct swim_drop_rate *dr, double rate, bool is_for_in,
+		      bool is_for_out)
+{
+	dr->is_for_in = is_for_in;
+	dr->is_for_out = is_for_out;
+	dr->rate = rate;
+}
+
+/**
+ * Drop components packet filter to drop packets containing
+ * specified SWIM components.
+ */
+struct swim_drop_components {
+	/** List of component body keys. */
+	const int *keys;
+	/** Length of @a keys. */
+	int key_count;
+};
+
+/** Initialize drop components packet filter. */
+static inline void
+swim_drop_components_create(struct swim_drop_components *dc, const int *keys,
+			    int key_count)
+{
+	dc->keys = keys;
+	dc->key_count = key_count;
+}
+
 /**
  * SWIM cluster node and its UUID. UUID is stored separately
  * because sometimes a test wants to drop a SWIM instance, but
@@ -52,6 +95,15 @@ struct swim_node {
 	 * that instance.
 	 */
 	struct tt_uuid uuid;
+	/**
+	 * Filter to drop packets with a certain probability
+	 * from/to a specified direction.
+	 */
+	struct swim_drop_rate drop_rate;
+	/**
+	 * Filter to drop packets with specified SWIM components.
+	 */
+	struct swim_drop_components drop_components;
 };
 
 /**
@@ -76,6 +128,24 @@ swim_cluster_id_to_uri(char *buffer, int id)
 	sprintf(buffer, "127.0.0.1:%d", id + 1);
 }
 
+/** Create a SWIM cluster node @a n with a 0-based @a id. */
+static inline void
+swim_node_create(struct swim_node *n, int id)
+{
+	n->swim = swim_new();
+	assert(n->swim != NULL);
+	char uri[128];
+	swim_cluster_id_to_uri(uri, id);
+	n->uuid = uuid_nil;
+	n->uuid.time_low = id + 1;
+	int rc = swim_cfg(n->swim, uri, -1, -1, -1, &n->uuid);
+	assert(rc == 0);
+	(void) rc;
+
+	swim_drop_rate_create(&n->drop_rate, 0, false, false);
+	swim_drop_components_create(&n->drop_components, NULL, 0);
+}
+
 struct swim_cluster *
 swim_cluster_new(int size)
 {
@@ -87,20 +157,9 @@ swim_cluster_new(int size)
 	res->size = size;
 	res->ack_timeout = -1;
 	res->gc_mode = SWIM_GC_DEFAULT;
-	struct tt_uuid uuid;
-	memset(&uuid, 0, sizeof(uuid));
-	char *uri = tt_static_buf();
 	struct swim_node *n = res->node;
-	for (int i = 0; i < size; ++i, ++n) {
-		n->swim = swim_new();
-		assert(n->swim != NULL);
-		swim_cluster_id_to_uri(uri, i);
-		uuid.time_low = i + 1;
-		n->uuid = uuid;
-		int rc = swim_cfg(n->swim, uri, -1, -1, -1, &uuid);
-		assert(rc == 0);
-		(void) rc;
-	}
+	for (int i = 0; i < size; ++i, ++n)
+		swim_node_create(n, i);
 	return res;
 }
 
@@ -271,16 +330,6 @@ swim_cluster_unblock_io(struct swim_cluster *cluster, int i)
 	swim_test_transport_unblock_fd(swim_fd(s));
 }
 
-/** A structure used by drop rate packet filter. */
-struct swim_drop_rate {
-	/** True if should be applied to incoming packets. */
-	bool is_for_in;
-	/** True if should be applied to outgoing packets. */
-	bool is_for_out;
-	/** Drop rate percentage. */
-	double rate;
-};
-
 /** Create a new drop rate filter helper. */
 static inline struct swim_drop_rate *
 swim_drop_rate_new(double rate, bool is_for_in, bool is_for_out)
@@ -315,14 +364,15 @@ static void
 swim_cluster_set_drop_generic(struct swim_cluster *cluster, int i,
 			      double value, bool is_for_in, bool is_for_out)
 {
-	int fd = swim_fd(swim_cluster_member(cluster, i));
+	struct swim_node *n = swim_cluster_node(cluster, i);
+	int fd = swim_fd(n->swim);
 	if (value == 0) {
 		swim_test_transport_remove_filter(fd, swim_filter_drop_rate);
 		return;
 	}
-	struct swim_drop_rate *dr = swim_drop_rate_new(value, is_for_in,
-						       is_for_out);
-	swim_test_transport_add_filter(fd, swim_filter_drop_rate, free, dr);
+	swim_drop_rate_create(&n->drop_rate, value, is_for_in, is_for_out);
+	swim_test_transport_add_filter(fd, swim_filter_drop_rate,
+				       &n->drop_rate);
 }
 
 void
@@ -343,16 +393,6 @@ swim_cluster_set_drop_in(struct swim_cluster *cluster, int i, double value)
 	swim_cluster_set_drop_generic(cluster, i, value, true, false);
 }
 
-/**
- * A list of components to drop used by component packet filter.
- */
-struct swim_drop_components {
-	/** List of component body keys. */
-	const int *keys;
-	/** Length of @a keys. */
-	int key_count;
-};
-
 /**
  * Check if a packet contains any of the components to filter out.
  */
@@ -381,19 +421,16 @@ void
 swim_cluster_drop_components(struct swim_cluster *cluster, int i,
 			     const int *keys, int key_count)
 {
-	int fd = swim_fd(swim_cluster_member(cluster, i));
+	struct swim_node *n = swim_cluster_node(cluster, i);
+	int fd = swim_fd(n->swim);
 	if (key_count == 0) {
 		swim_test_transport_remove_filter(fd,
 						  swim_filter_drop_component);
 		return;
 	}
-	struct swim_drop_components *dc =
-		(struct swim_drop_components *) malloc(sizeof(*dc));
-	assert(dc != NULL);
-	dc->key_count = key_count;
-	dc->keys = keys;
-	swim_test_transport_add_filter(fd, swim_filter_drop_component, free,
-				       dc);
+	swim_drop_components_create(&n->drop_components, keys, key_count);
+	swim_test_transport_add_filter(fd, swim_filter_drop_component,
+				       &n->drop_components);
 }
 
 /** Check if @a s1 knows every member of @a s2's table. */
-- 
2.20.1 (Apple Git-117)