[PATCH v2 1/3] vclock: use static buffer to format vclock

Vladimir Davydov vdavydov.dev at gmail.com
Fri Feb 15 15:25:47 MSK 2019


Currently, vclock_to_string() allocates the formatted vclock string
using malloc() and hence the caller is responsible for freeing it, which
isn't very user-friendly. Let's use a static buffer as we do to format
other objects.
---
 src/box/error.cc       |  6 ++---
 src/box/gc.c           |  6 ++---
 src/box/replication.cc | 12 ++++------
 src/box/vclock.c       | 64 +++++++++++++-------------------------------------
 src/box/vclock.h       |  5 ++--
 src/box/xlog.c         | 16 ++++---------
 test/unit/vclock.cc    |  3 +--
 7 files changed, 31 insertions(+), 81 deletions(-)

diff --git a/src/box/error.cc b/src/box/error.cc
index aa81a390..47dce330 100644
--- a/src/box/error.cc
+++ b/src/box/error.cc
@@ -183,14 +183,12 @@ XlogGapError::XlogGapError(const char *file, unsigned line,
 			   const struct vclock *from, const struct vclock *to)
 		: XlogError(&type_XlogGapError, file, line)
 {
-	char *s_from = vclock_to_string(from);
-	char *s_to = vclock_to_string(to);
+	const char *s_from = vclock_to_string(from);
+	const char *s_to = vclock_to_string(to);
 	snprintf(errmsg, sizeof(errmsg),
 		 "Missing .xlog file between LSN %lld %s and %lld %s",
 		 (long long) vclock_sum(from), s_from ? s_from : "",
 		 (long long) vclock_sum(to), s_to ? s_to : "");
-	free(s_from);
-	free(s_to);
 }
 
 struct error *
diff --git a/src/box/gc.c b/src/box/gc.c
index 7411314a..5639edd8 100644
--- a/src/box/gc.c
+++ b/src/box/gc.c
@@ -294,10 +294,8 @@ gc_advance(const struct vclock *vclock)
 		consumer->is_inactive = true;
 		gc_tree_remove(&gc.consumers, consumer);
 
-		char *vclock_str = vclock_to_string(&consumer->vclock);
-		say_crit("deactivated WAL consumer %s at %s",
-			 consumer->name, vclock_str);
-		free(vclock_str);
+		say_crit("deactivated WAL consumer %s at %s", consumer->name,
+			 vclock_to_string(&consumer->vclock));
 
 		consumer = next;
 	}
diff --git a/src/box/replication.cc b/src/box/replication.cc
index 2cb4ec0f..19ad5026 100644
--- a/src/box/replication.cc
+++ b/src/box/replication.cc
@@ -686,9 +686,9 @@ replicaset_needs_rejoin(struct replica **master)
 		const char *uuid_str = tt_uuid_str(&replica->uuid);
 		const char *addr_str = sio_strfaddr(&applier->addr,
 						applier->addr_len);
-		char *local_vclock_str = vclock_to_string(&replicaset.vclock);
-		char *remote_vclock_str = vclock_to_string(&ballot->vclock);
-		char *gc_vclock_str = vclock_to_string(&ballot->gc_vclock);
+		const char *local_vclock_str = vclock_to_string(&replicaset.vclock);
+		const char *remote_vclock_str = vclock_to_string(&ballot->vclock);
+		const char *gc_vclock_str = vclock_to_string(&ballot->gc_vclock);
 
 		say_info("can't follow %s at %s: required %s available %s",
 			 uuid_str, addr_str, local_vclock_str, gc_vclock_str);
@@ -703,7 +703,7 @@ replicaset_needs_rejoin(struct replica **master)
 				 "replica has local rows: local %s remote %s",
 				 uuid_str, addr_str, local_vclock_str,
 				 remote_vclock_str);
-			goto next;
+			continue;
 		}
 
 		/* Prefer a master with the max vclock. */
@@ -711,10 +711,6 @@ replicaset_needs_rejoin(struct replica **master)
 		    vclock_sum(&ballot->vclock) >
 		    vclock_sum(&leader->applier->ballot.vclock))
 			leader = replica;
-next:
-		free(local_vclock_str);
-		free(remote_vclock_str);
-		free(gc_vclock_str);
 	}
 	if (leader == NULL)
 		return false;
diff --git a/src/box/vclock.c b/src/box/vclock.c
index d4b2ba75..cea67017 100644
--- a/src/box/vclock.c
+++ b/src/box/vclock.c
@@ -35,6 +35,7 @@
 #include <ctype.h>
 
 #include "diag.h"
+#include "trivia/util.h"
 
 int64_t
 vclock_follow(struct vclock *vclock, uint32_t replica_id, int64_t lsn)
@@ -50,64 +51,31 @@ vclock_follow(struct vclock *vclock, uint32_t replica_id, int64_t lsn)
 	return prev_lsn;
 }
 
-CFORMAT(printf, 4, 0) static inline int
-rsnprintf(char **buf, char **pos, char **end, const char *fmt, ...)
+static int
+vclock_snprint(char *buf, int size, const struct vclock *vclock)
 {
-	int rc = 0;
-	va_list ap;
-
-	while (1) {
-		va_start(ap, fmt);
-		int n = vsnprintf(*pos, *end - *pos, fmt, ap);
-		va_end(ap);
-		assert(n > -1); /* glibc >= 2.0.6, see vsnprintf(3) */
-		if (n < *end - *pos) {
-			*pos += n;
-			break;
-		}
-
-		/* Reallocate buffer */
-		ptrdiff_t cap = (*end - *buf) > 0 ? (*end - *buf) : 32;
-		while (cap <= *pos - *buf + n)
-			cap *= 2;
-		char *chunk = (char *) realloc(*buf, cap);
-		if (chunk == NULL) {
-			diag_set(OutOfMemory, cap, "malloc", "vclock");
-			free(*buf);
-			*buf = *end = *pos = NULL;
-			rc = -1;
-			break;
-		}
-		*pos = chunk + (*pos - *buf);
-		*end = chunk + cap;
-		*buf = chunk;
-	}
-
-	return rc;
-}
-
-char *
-vclock_to_string(const struct vclock *vclock)
-{
-	(void) vclock;
-	char *buf = NULL, *pos = NULL, *end = NULL;
-
-	if (rsnprintf(&buf, &pos, &end, "{") != 0)
-		return NULL;
+	int total = 0;
+	SNPRINT(total, snprintf, buf, size, "{");
 
 	const char *sep = "";
 	struct vclock_iterator it;
 	vclock_iterator_init(&it, vclock);
 	vclock_foreach(&it, replica) {
-		if (rsnprintf(&buf, &pos, &end, "%s%u: %lld", sep,
-			      replica.id, (long long) replica.lsn) != 0)
-			return NULL;
+		SNPRINT(total, snprintf, buf, size, "%s%u: %lld",
+			sep, (unsigned)replica.id, (long long)replica.lsn);
 		sep = ", ";
 	}
 
-	if (rsnprintf(&buf, &pos, &end, "}") != 0)
-		return NULL;
+	SNPRINT(total, snprintf, buf, size, "}");
+	return total;
+}
 
+const char *
+vclock_to_string(const struct vclock *vclock)
+{
+	char *buf = tt_static_buf();
+	if (vclock_snprint(buf, TT_STATIC_BUF_LEN, vclock) < 0)
+		return "<failed to format vclock>";
 	return buf;
 }
 
diff --git a/src/box/vclock.h b/src/box/vclock.h
index 0c999690..1a174c1e 100644
--- a/src/box/vclock.h
+++ b/src/box/vclock.h
@@ -245,10 +245,9 @@ vclock_merge(struct vclock *dst, struct vclock *diff)
  * \brief Format vclock to YAML-compatible string representation:
  * { replica_id: lsn, replica_id:lsn })
  * \param vclock vclock
- * \return fomatted string. This pointer should be passed to free(3) to
- * release the allocated storage when it is no longer needed.
+ * \return fomatted string, stored in a static buffer.
  */
-char *
+const char *
 vclock_to_string(const struct vclock *vclock);
 
 /**
diff --git a/src/box/xlog.c b/src/box/xlog.c
index bd5614f6..d5de8e6d 100644
--- a/src/box/xlog.c
+++ b/src/box/xlog.c
@@ -151,20 +151,12 @@ xlog_meta_format(const struct xlog_meta *meta, char *buf, int size)
 		meta->filetype, v13, PACKAGE_VERSION,
 		tt_uuid_str(&meta->instance_uuid));
 	if (vclock_is_set(&meta->vclock)) {
-		char *vstr = vclock_to_string(&meta->vclock);
-		if (vstr == NULL)
-			return -1;
-		SNPRINT(total, snprintf, buf, size,
-			VCLOCK_KEY ": %s\n", vstr);
-		free(vstr);
+		SNPRINT(total, snprintf, buf, size, VCLOCK_KEY ": %s\n",
+			vclock_to_string(&meta->vclock));
 	}
 	if (vclock_is_set(&meta->prev_vclock)) {
-		char *vstr = vclock_to_string(&meta->prev_vclock);
-		if (vstr == NULL)
-			return -1;
-		SNPRINT(total, snprintf, buf, size,
-			PREV_VCLOCK_KEY ": %s\n", vstr);
-		free(vstr);
+		SNPRINT(total, snprintf, buf, size, PREV_VCLOCK_KEY ": %s\n",
+			vclock_to_string(&meta->prev_vclock));
 	}
 	SNPRINT(total, snprintf, buf, size, "\n");
 	assert(total > 0);
diff --git a/test/unit/vclock.cc b/test/unit/vclock.cc
index 6a1d3bc2..15e9f937 100644
--- a/test/unit/vclock.cc
+++ b/test/unit/vclock.cc
@@ -249,11 +249,10 @@ test_tostring_one(uint32_t count, const int64_t *lsns, const char *res)
 		if (lsns[node_id] > 0)
 			vclock_follow(&vclock, node_id, lsns[node_id]);
 	}
-	char *str = vclock_to_string(&vclock);
+	const char *str = vclock_to_string(&vclock);
 	int result = strcmp(str, res);
 	if (result)
 		diag("\n!!!new result!!! %s\n", str);
-	free(str);
 	return !result;
 }
 
-- 
2.11.0




More information about the Tarantool-patches mailing list