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
