From 85bcc601653ba29fc360978866c0af46f07af8ac Mon Sep 17 00:00:00 2001 From: Egor Seredin <4819888+agmt@users.noreply.github.com> Date: Wed, 4 Nov 2020 20:38:46 +0900 Subject: [PATCH] Allow '\0' inside of result of sdscatvprintf, and efficiency improvements (#6260) This will allow to use: RedisModule_CreateStringPrintf(ctx, "%s %c %s", "string1", 0, "string2"); On large string, the previous code would incrementally retry to double the output buffer. now it uses the the return value of snprintf and grows to the right size in one step. and also avoids an excessive strlen in sdscat at the end. (cherry picked from commit 1ad4e18394eac68ab8bae1ef5a2920086ce0f9ba) --- src/sds.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/sds.c b/src/sds.c index 965badf13..dc664ca9b 100644 --- a/src/sds.c +++ b/src/sds.c @@ -523,6 +523,7 @@ sds sdscatvprintf(sds s, const char *fmt, va_list ap) { va_list cpy; char staticbuf[1024], *buf = staticbuf, *t; size_t buflen = strlen(fmt)*2; + int bufstrlen; /* We try to start using a static buffer for speed. * If not possible we revert to heap allocation. */ @@ -533,16 +534,19 @@ sds sdscatvprintf(sds s, const char *fmt, va_list ap) { buflen = sizeof(staticbuf); } - /* Try with buffers two times bigger every time we fail to + /* Alloc enough space for buffer and \0 after failing to * fit the string in the current buffer size. */ while(1) { - buf[buflen-2] = '\0'; va_copy(cpy,ap); - vsnprintf(buf, buflen, fmt, cpy); + bufstrlen = vsnprintf(buf, buflen, fmt, cpy); va_end(cpy); - if (buf[buflen-2] != '\0') { + if (bufstrlen < 0) { if (buf != staticbuf) s_free(buf); - buflen *= 2; + return NULL; + } + if (((size_t)bufstrlen) >= buflen) { + if (buf != staticbuf) s_free(buf); + buflen = ((size_t)bufstrlen) + 1; buf = s_malloc(buflen); if (buf == NULL) return NULL; continue; @@ -551,7 +555,7 @@ sds sdscatvprintf(sds s, const char *fmt, va_list ap) { } /* Finally concat the obtained string to the SDS string and return it. */ - t = sdscat(s, buf); + t = sdscatlen(s, buf, bufstrlen); if (buf != staticbuf) s_free(buf); return t; } @@ -1159,6 +1163,22 @@ int sdsTest(void) { test_cond("sdscatprintf() seems working in the base case", sdslen(x) == 3 && memcmp(x,"123\0",4) == 0) + sdsfree(x); + x = sdscatprintf(sdsempty(),"a%cb",0); + test_cond("sdscatprintf() seems working with \\0 inside of result", + sdslen(x) == 3 && memcmp(x,"a\0""b\0",4) == 0) + + { + sdsfree(x); + char etalon[1024*1024]; + for (size_t i = 0; i < sizeof(etalon); i++) { + etalon[i] = '0'; + } + x = sdscatprintf(sdsempty(),"%0*d",(int)sizeof(etalon),0); + test_cond("sdscatprintf() can print 1MB", + sdslen(x) == sizeof(etalon) && memcmp(x,etalon,sizeof(etalon)) == 0) + } + sdsfree(x); x = sdsnew("--"); x = sdscatfmt(x, "Hello %s World %I,%I--", "Hi!", LLONG_MIN,LLONG_MAX);