From: Quentin Young Date: Fri, 3 Feb 2017 21:43:59 +0000 (+0000) Subject: Restore sprintbuf(), add macro for string literals X-Git-Tag: json-c-0.13-20171207~104^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f6f852fd9367a815a711b5097139002e7385b165;p=json-c Restore sprintbuf(), add macro for string literals Hawciz pointed out that the previous commit modifies the public interface of printbuf. Per his suggestion, sprintbuf() was restored and a new pair of macros was added that wraps printbuf_memappend(). Using a wrapper macro instead of modifying sprintbuf() also reduces function call overhead, bringing total performance gains to approximately 400%. --- diff --git a/json_object.c b/json_object.c index 2d688ce..076cf45 100644 --- a/json_object.c +++ b/json_object.c @@ -149,11 +149,10 @@ static int json_escape_str(struct printbuf *pb, const char *str, int len, int fl pos - start_offset); static char sbuf[7]; snprintf(sbuf, sizeof(sbuf), - "\\u00%c%c", - json_hex_chars[c >> 4], - json_hex_chars[c & 0xf]); - printbuf_memappend (pb, sbuf, sizeof(sbuf) - 1); - + "\\u00%c%c", + json_hex_chars[c >> 4], + json_hex_chars[c & 0xf]); + printbuf_memappend_fast(pb, sbuf, (int) sizeof(sbuf) - 1); start_offset = ++pos; } else pos++; @@ -364,29 +363,29 @@ static int json_object_object_to_json_string(struct json_object* jso, int had_children = 0; struct json_object_iter iter; - sprintbuf(pb, "{" /*}*/); + printbuf_strappend(pb, "{" /*}*/); if (flags & JSON_C_TO_STRING_PRETTY) - sprintbuf(pb, "\n"); + printbuf_strappend(pb, "\n"); json_object_object_foreachC(jso, iter) { if (had_children) { - sprintbuf(pb, ","); + printbuf_strappend(pb, ","); if (flags & JSON_C_TO_STRING_PRETTY) - sprintbuf(pb, "\n"); + printbuf_strappend(pb, "\n"); } had_children = 1; if (flags & JSON_C_TO_STRING_SPACED) - sprintbuf(pb, " "); + printbuf_strappend(pb, " "); indent(pb, level+1, flags); - sprintbuf(pb, "\""); + printbuf_strappend(pb, "\""); json_escape_str(pb, iter.key, strlen(iter.key), flags); if (flags & JSON_C_TO_STRING_SPACED) - sprintbuf(pb, "\": "); + printbuf_strappend(pb, "\": "); else - sprintbuf(pb, "\":"); + printbuf_strappend(pb, "\":"); if(iter.val == NULL) - sprintbuf(pb, "null"); + printbuf_strappend(pb, "null"); else if (iter.val->_to_json_string(iter.val, pb, level+1,flags) < 0) return -1; @@ -394,13 +393,13 @@ static int json_object_object_to_json_string(struct json_object* jso, if (flags & JSON_C_TO_STRING_PRETTY) { if (had_children) - sprintbuf(pb, "\n"); + printbuf_strappend(pb, "\n"); indent(pb,level,flags); } if (flags & JSON_C_TO_STRING_SPACED) - return sprintbuf(pb, /*{*/ " }"); + return printbuf_strappend(pb, /*{*/ " }"); else - return sprintbuf(pb, /*{*/ "}"); + return printbuf_strappend(pb, /*{*/ "}"); } @@ -541,8 +540,8 @@ static int json_object_boolean_to_json_string(struct json_object* jso, int flags) { if (jso->o.c_boolean) - return sprintbuf(pb, "true"); - return sprintbuf(pb, "false"); + return printbuf_strappend(pb, "true"); + return printbuf_strappend(pb, "false"); } struct json_object* json_object_new_boolean(json_bool b) @@ -592,7 +591,7 @@ static int json_object_int_to_json_string(struct json_object* jso, /* room for 19 digits, the sign char, and a null term */ static char sbuf[21]; snprintf(sbuf, sizeof(sbuf), "%"PRId64, jso->o.c_int64); - return sprintbuf(pb, sbuf); + return printbuf_memappend (pb, sbuf, strlen(sbuf)); } struct json_object* json_object_new_int(int32_t i) @@ -860,9 +859,9 @@ static int json_object_string_to_json_string(struct json_object* jso, int level, int flags) { - sprintbuf(pb, "\""); + printbuf_strappend(pb, "\""); json_escape_str(pb, get_string_component(jso), jso->o.c_string.len, flags); - sprintbuf(pb, "\""); + printbuf_strappend(pb, "\""); return 0; } @@ -979,25 +978,25 @@ static int json_object_array_to_json_string(struct json_object* jso, int had_children = 0; size_t ii; - sprintbuf(pb, "["); + printbuf_strappend(pb, "["); if (flags & JSON_C_TO_STRING_PRETTY) - sprintbuf(pb, "\n"); + printbuf_strappend(pb, "\n"); for(ii=0; ii < json_object_array_length(jso); ii++) { struct json_object *val; if (had_children) { - sprintbuf(pb, ","); + printbuf_strappend(pb, ","); if (flags & JSON_C_TO_STRING_PRETTY) - sprintbuf(pb, "\n"); + printbuf_strappend(pb, "\n"); } had_children = 1; if (flags & JSON_C_TO_STRING_SPACED) - sprintbuf(pb, " "); + printbuf_strappend(pb, " "); indent(pb, level + 1, flags); val = json_object_array_get_idx(jso, ii); if(val == NULL) - sprintbuf(pb, "null"); + printbuf_strappend(pb, "null"); else if (val->_to_json_string(val, pb, level+1, flags) < 0) return -1; @@ -1005,13 +1004,13 @@ static int json_object_array_to_json_string(struct json_object* jso, if (flags & JSON_C_TO_STRING_PRETTY) { if (had_children) - sprintbuf(pb, "\n"); + printbuf_strappend(pb, "\n"); indent(pb,level,flags); } if (flags & JSON_C_TO_STRING_SPACED) - return sprintbuf(pb, " ]"); - return sprintbuf(pb, "]"); + return printbuf_strappend(pb, " ]"); + return printbuf_strappend(pb, "]"); } static void json_object_array_entry_free(void *data) diff --git a/printbuf.c b/printbuf.c index 2d52ddd..2745339 100644 --- a/printbuf.c +++ b/printbuf.c @@ -85,7 +85,6 @@ int printbuf_memappend(struct printbuf *p, const char *buf, int size) if (printbuf_extend(p, p->bpos + size + 1) < 0) return -1; } - memcpy(p->buf + p->bpos, buf, size); p->bpos += size; p->buf[p->bpos]= '\0'; @@ -112,6 +111,34 @@ int printbuf_memset(struct printbuf *pb, int offset, int charvalue, int len) return 0; } +int sprintbuf(struct printbuf *p, const char *msg, ...) +{ + va_list ap; + char *t; + int size; + char buf[128]; + + /* user stack buffer first */ + va_start(ap, msg); + size = vsnprintf(buf, 128, msg, ap); + va_end(ap); + /* if string is greater than stack buffer, then use dynamic string + with vasprintf. Note: some implementation of vsnprintf return -1 + if output is truncated whereas some return the number of bytes that + would have been written - this code handles both cases. */ + if(size == -1 || size > 127) { + va_start(ap, msg); + if((size = vasprintf(&t, msg, ap)) < 0) { va_end(ap); return -1; } + va_end(ap); + printbuf_memappend(p, t, size); + free(t); + return size; + } else { + printbuf_memappend(p, buf, size); + return size; + } +} + void printbuf_reset(struct printbuf *p) { p->buf[0] = '\0'; @@ -125,8 +152,3 @@ void printbuf_free(struct printbuf *p) free(p); } } - -inline int sprintbuf(struct printbuf *p, const char *buf) -{ - return printbuf_memappend(p, buf, strlen(buf)); -} diff --git a/printbuf.h b/printbuf.h index d8c2438..68ac949 100644 --- a/printbuf.h +++ b/printbuf.h @@ -29,12 +29,13 @@ struct printbuf { extern struct printbuf* printbuf_new(void); -/* As an optimization, printbuf_memappend_fast is defined as a macro +/* As an optimization, printbuf_memappend_fast() is defined as a macro * that handles copying data if the buffer is large enough; otherwise - * it invokes printbuf_memappend_real() which performs the heavy + * it invokes printbuf_memappend() which performs the heavy * lifting of realloc()ing the buffer and copying data. - * Your code should not use printbuf_memappend directly--use - * printbuf_memappend_fast instead. + * + * Your code should not use printbuf_memappend() directly unless it + * checks the return code. Use printbuf_memappend_fast() instead. */ extern int printbuf_memappend(struct printbuf *p, const char *buf, int size); @@ -50,6 +51,28 @@ do { \ #define printbuf_length(p) ((p)->bpos) +/** + * Results in a compile error if the argument is not a string literal. + */ +#define _printbuf_check_literal(mystr) ("" mystr) + +/** + * This is an optimization wrapper around printbuf_memappend() that is useful + * for appending string literals. Since the size of string constants is known + * at compile time, using this macro can avoid a costly strlen() call. This is + * especially helpful when a constant string must be appended many times. If + * you got here because of a compilation error caused by passing something + * other than a string literal, use printbuf_memappend_fast() in conjunction + * with strlen(). + * + * See also: + * printbuf_memappend_fast() + * printbuf_memappend() + * sprintbuf() + */ +#define printbuf_strappend(pb, str) \ + printbuf_memappend ((pb), _printbuf_check_literal(str), sizeof(str) - 1) + /** * Set len bytes of the buffer to charvalue, starting at offset offset. * Similar to calling memset(x, charvalue, len); @@ -61,8 +84,22 @@ do { \ extern int printbuf_memset(struct printbuf *pb, int offset, int charvalue, int len); +/** + * Formatted print to printbuf. + * + * This function is the most expensive of the available functions for appending + * string data to a printbuf and should be used only where convenience is more + * important than speed. Avoid using this function in high performance code or + * tight loops; in these scenarios, consider using snprintf() with a static + * buffer in conjunction with one of the printbuf_*append() functions. + * + * See also: + * printbuf_memappend_fast() + * printbuf_memappend() + * printbuf_strappend() + */ extern int -sprintbuf(struct printbuf *p, const char *msg); +sprintbuf(struct printbuf *p, const char *msg, ...); extern void printbuf_reset(struct printbuf *p); diff --git a/tests/test_printbuf.c b/tests/test_printbuf.c index 143ff4a..0dd2dc9 100644 --- a/tests/test_printbuf.c +++ b/tests/test_printbuf.c @@ -17,7 +17,7 @@ static void test_basic_printbuf_memset() printf("%s: starting test\n", __func__); pb = printbuf_new(); - sprintbuf(pb, "blue:1"); + sprintbuf(pb, "blue:%d", 1); printbuf_memset(pb, -1, 'x', 52); printf("Buffer contents:%.*s\n", printbuf_length(pb), pb->buf); printbuf_free(pb); @@ -106,6 +106,49 @@ static void test_printbuf_memappend(int *before_resize) printf("Append to just after resize: %d, [%s]\n", printbuf_length(pb), pb->buf); free(data); + printbuf_free(pb); + +#define SA_TEST_STR "XXXXXXXXXXXXXXXX" + pb = printbuf_new(); + printbuf_strappend(pb, SA_TEST_STR); + printf("Buffer size after printbuf_strappend(): %d, [%s]\n", printbuf_length(pb), pb->buf); + printbuf_free(pb); +#undef SA_TEST_STR + + printf("%s: end test\n", __func__); +} + +static void test_sprintbuf(int before_resize); +static void test_sprintbuf(int before_resize) +{ + struct printbuf *pb; + + printf("%s: starting test\n", __func__); + pb = printbuf_new(); + printf("Buffer length: %d\n", printbuf_length(pb)); + + char *data = malloc(before_resize + 1 + 1); + memset(data, 'X', before_resize + 1 + 1); + data[before_resize + 1] = '\0'; + sprintbuf(pb, "%s", data); + free(data); + printf("sprintbuf to just after resize(%d+1): %d, [%s], strlen(buf)=%d\n", before_resize, printbuf_length(pb), pb->buf, (int)strlen(pb->buf)); + + printbuf_reset(pb); + sprintbuf(pb, "plain"); + printf("%d, [%s]\n", printbuf_length(pb), pb->buf); + + sprintbuf(pb, "%d", 1); + printf("%d, [%s]\n", printbuf_length(pb), pb->buf); + + sprintbuf(pb, "%d", INT_MAX); + printf("%d, [%s]\n", printbuf_length(pb), pb->buf); + + sprintbuf(pb, "%d", INT_MIN); + printf("%d, [%s]\n", printbuf_length(pb), pb->buf); + + sprintbuf(pb, "%s", "%s"); + printf("%d, [%s]\n", printbuf_length(pb), pb->buf); printbuf_free(pb); printf("%s: end test\n", __func__); @@ -123,6 +166,8 @@ int main(int argc, char **argv) printf("========================================\n"); test_printbuf_memappend(&before_resize); printf("========================================\n"); + test_sprintbuf(before_resize); + printf("========================================\n"); return 0; } diff --git a/tests/test_printbuf.expected b/tests/test_printbuf.expected index a3b107d..6464412 100644 --- a/tests/test_printbuf.expected +++ b/tests/test_printbuf.expected @@ -18,5 +18,16 @@ Partial append: 3, [blu] With embedded \0 character: 4, [ab] Append to just before resize: 31, [XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX] Append to just after resize: 32, [XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX] +Buffer size after printbuf_strappend(): 16, [XXXXXXXXXXXXXXXX] test_printbuf_memappend: end test ======================================== +test_sprintbuf: starting test +Buffer length: 0 +sprintbuf to just after resize(31+1): 32, [XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX], strlen(buf)=32 +5, [plain] +6, [plain1] +16, [plain12147483647] +27, [plain12147483647-2147483648] +29, [plain12147483647-2147483648%s] +test_sprintbuf: end test +========================================