From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH v2 1/3] vclock: use static buffer to format vclock Date: Fri, 15 Feb 2019 15:25:47 +0300 Message-Id: <61f47446fb76bbcb80ad003c209bd697b1bff007.1550232829.git.vdavydov.dev@gmail.com> In-Reply-To: References: In-Reply-To: References: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: 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 #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 ""; 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