From 4ac55be9b58b48a602ba23f536092eb644775f05 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Sat, 9 Jul 2011 14:54:01 +0200 Subject: Update printf-formatting tests to fail --- test.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test.c b/test.c index bac10a7..d3a3dbd 100644 --- a/test.c +++ b/test.c @@ -142,16 +142,18 @@ static void test_format_commands(void) { len == 4+4+(3+2)+4+(1+2)+4+(1+2)); free(cmd); + /* sizeof(long long) is 8 bytes regardless of architecture */ test("Format command with printf-delegation (long long): "); - len = redisFormatCommand(&cmd,"key:%08lld",1234ll); - test_cond(strncmp(cmd,"*1\r\n$12\r\nkey:00001234\r\n",len) == 0 && - len == 4+5+(12+2)); + len = redisFormatCommand(&cmd,"key:%08lld str:%s",1234ll, "hello"); + test_cond(strncmp(cmd,"*2\r\n$12\r\nkey:00001234\r\n$9\r\nstr:hello\r\n",len) == 0 && + len == 4+5+(12+2)+4+(9+2)); free(cmd); + /* sizeof(float) is 4 bytes regardless of architecture */ test("Format command with printf-delegation (float): "); - len = redisFormatCommand(&cmd,"v:%06.1f",12.34f); - test_cond(strncmp(cmd,"*1\r\n$8\r\nv:0012.3\r\n",len) == 0 && - len == 4+4+(8+2)); + len = redisFormatCommand(&cmd,"v:%06.1f str:%s",12.34f,"hello"); + test_cond(strncmp(cmd,"*2\r\n$8\r\nv:0012.3\r\n$9\r\nstr:hello\r\n",len) == 0 && + len == 4+4+(8+2)+4+(9+2)); free(cmd); test("Format command with printf-delegation and extra interpolation: "); -- cgit v1.2.3 From 27c96dde77e9a36602cc3c5040375b081652a4bf Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Sat, 9 Jul 2011 15:03:00 +0200 Subject: Use correct type when calling va_arg in formatter --- hiredis.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 67 insertions(+), 19 deletions(-) diff --git a/hiredis.c b/hiredis.c index 9ea998a..83218cd 100644 --- a/hiredis.c +++ b/hiredis.c @@ -770,33 +770,81 @@ int redisvFormatCommand(char **target, const char *format, va_list ap) { while (*_p != '\0' && isdigit(*_p)) _p++; } - /* Modifiers */ - if (*_p != '\0') { - if (*_p == 'h' || *_p == 'l') { - /* Allow a single repetition for these modifiers */ - if (_p[0] == _p[1]) _p++; - _p++; + /* Copy va_list before consuming with va_arg */ + va_copy(_cpy,ap); + + /* Integer conversion (without modifiers) */ + if (strchr("diouxX",*_p) != NULL) { + va_arg(ap,int); + goto fmt_valid; + } + + /* Double conversion (without modifiers) */ + if (strchr("eEfFgGaA",*_p) != NULL) { + va_arg(ap,double); + goto fmt_valid; + } + + /* Size: char */ + if (_p[0] == 'h' && _p[1] == 'h') { + _p += 2; + if (*_p != '\0' && strchr("diouxX",*_p) != NULL) { + va_arg(ap,int); /* char gets promoted to int */ + goto fmt_valid; } + goto fmt_invalid; } - /* Conversion specifier */ - if (*_p != '\0' && strchr("diouxXeEfFgGaA",*_p) != NULL) { - _l = (_p+1)-c; - if (_l < sizeof(_format)-2) { - memcpy(_format,c,_l); - _format[_l] = '\0'; - va_copy(_cpy,ap); - newarg = sdscatvprintf(curarg,_format,_cpy); - va_end(_cpy); - - /* Update current position (note: outer blocks - * increment c twice so compensate here) */ - c = _p-1; + /* Size: short */ + if (_p[0] == 'h') { + _p += 1; + if (*_p != '\0' && strchr("diouxX",*_p) != NULL) { + va_arg(ap,int); /* short gets promoted to int */ + goto fmt_valid; } + goto fmt_invalid; } + /* Size: long long */ + if (_p[0] == 'l' && _p[1] == 'l') { + _p += 2; + if (*_p != '\0' && strchr("diouxX",*_p) != NULL) { + va_arg(ap,long long); + goto fmt_valid; + } + goto fmt_invalid; + } + + /* Size: long */ + if (_p[0] == 'l') { + _p += 1; + if (*_p != '\0' && strchr("diouxX",*_p) != NULL) { + va_arg(ap,long); + goto fmt_valid; + } + goto fmt_invalid; + } + + fmt_invalid: /* Consume and discard vararg */ va_arg(ap,void); + va_end(_cpy); + break; + + fmt_valid: + _l = (_p+1)-c; + if (_l < sizeof(_format)-2) { + memcpy(_format,c,_l); + _format[_l] = '\0'; + newarg = sdscatvprintf(curarg,_format,_cpy); + + /* Update current position (note: outer blocks + * increment c twice so compensate here) */ + c = _p-1; + } + + va_end(_cpy); + break; } } -- cgit v1.2.3 From 2da784ce8f911f0eb1ef83fbcf3e2ce2d7529d15 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Sat, 9 Jul 2011 15:05:53 +0200 Subject: Abort on invalid format There is no way we can guess the width of the argument when we cannot infer its type from the format specifier. --- hiredis.c | 4 +--- test.c | 6 ++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/hiredis.c b/hiredis.c index 83218cd..dfee705 100644 --- a/hiredis.c +++ b/hiredis.c @@ -826,10 +826,8 @@ int redisvFormatCommand(char **target, const char *format, va_list ap) { } fmt_invalid: - /* Consume and discard vararg */ - va_arg(ap,void); va_end(_cpy); - break; + goto err; fmt_valid: _l = (_p+1)-c; diff --git a/test.c b/test.c index d3a3dbd..0dae70a 100644 --- a/test.c +++ b/test.c @@ -162,11 +162,9 @@ static void test_format_commands(void) { len == 4+4+(8+2)+4+(3+2)); free(cmd); - test("Format command with wrong printf format and extra interpolation: "); + test("Format command with invalid printf format: "); len = redisFormatCommand(&cmd,"key:%08p %b",1234,"foo",3); - test_cond(strncmp(cmd,"*2\r\n$6\r\nkey:8p\r\n$3\r\nfoo\r\n",len) == 0 && - len == 4+4+(6+2)+4+(3+2)); - free(cmd); + test_cond(len == -1); const char *argv[3]; argv[0] = "SET"; -- cgit v1.2.3 From dad240f0a364829a9969d864de5121f33d7332c2 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Sun, 10 Jul 2011 17:22:36 +0200 Subject: Test all supported types for printf-like formatting --- test.c | 52 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/test.c b/test.c index 0dae70a..83d265e 100644 --- a/test.c +++ b/test.c @@ -142,25 +142,39 @@ static void test_format_commands(void) { len == 4+4+(3+2)+4+(1+2)+4+(1+2)); free(cmd); - /* sizeof(long long) is 8 bytes regardless of architecture */ - test("Format command with printf-delegation (long long): "); - len = redisFormatCommand(&cmd,"key:%08lld str:%s",1234ll, "hello"); - test_cond(strncmp(cmd,"*2\r\n$12\r\nkey:00001234\r\n$9\r\nstr:hello\r\n",len) == 0 && - len == 4+5+(12+2)+4+(9+2)); - free(cmd); - - /* sizeof(float) is 4 bytes regardless of architecture */ - test("Format command with printf-delegation (float): "); - len = redisFormatCommand(&cmd,"v:%06.1f str:%s",12.34f,"hello"); - test_cond(strncmp(cmd,"*2\r\n$8\r\nv:0012.3\r\n$9\r\nstr:hello\r\n",len) == 0 && - len == 4+4+(8+2)+4+(9+2)); - free(cmd); - - test("Format command with printf-delegation and extra interpolation: "); - len = redisFormatCommand(&cmd,"key:%d %b",1234,"foo",3); - test_cond(strncmp(cmd,"*2\r\n$8\r\nkey:1234\r\n$3\r\nfoo\r\n",len) == 0 && - len == 4+4+(8+2)+4+(3+2)); - free(cmd); + /* Vararg width depends on the type. These tests make sure that the + * width is correctly determined using the format and subsequent varargs + * can correctly be interpolated. */ +#define INTEGER_WIDTH_TEST(fmt, type) do { \ + type value = 123; \ + test("Format command with printf-delegation (" #type "): "); \ + len = redisFormatCommand(&cmd,"key:%08" fmt " str:%s", value, "hello"); \ + test_cond(strncmp(cmd,"*2\r\n$12\r\nkey:00000123\r\n$9\r\nstr:hello\r\n",len) == 0 && \ + len == 4+5+(12+2)+4+(9+2)); \ + free(cmd); \ +} while(0) + +#define FLOAT_WIDTH_TEST(type) do { \ + type value = 123.0; \ + test("Format command with printf-delegation (" #type "): "); \ + len = redisFormatCommand(&cmd,"key:%08.3f str:%s", value, "hello"); \ + test_cond(strncmp(cmd,"*2\r\n$12\r\nkey:0123.000\r\n$9\r\nstr:hello\r\n",len) == 0 && \ + len == 4+5+(12+2)+4+(9+2)); \ + free(cmd); \ +} while(0) + + INTEGER_WIDTH_TEST("d", int); + INTEGER_WIDTH_TEST("hhd", char); + INTEGER_WIDTH_TEST("hd", short); + INTEGER_WIDTH_TEST("ld", long); + INTEGER_WIDTH_TEST("lld", long long); + INTEGER_WIDTH_TEST("u", unsigned int); + INTEGER_WIDTH_TEST("hhu", unsigned char); + INTEGER_WIDTH_TEST("hu", unsigned short); + INTEGER_WIDTH_TEST("lu", unsigned long); + INTEGER_WIDTH_TEST("llu", unsigned long long); + FLOAT_WIDTH_TEST(float); + FLOAT_WIDTH_TEST(double); test("Format command with invalid printf format: "); len = redisFormatCommand(&cmd,"key:%08p %b",1234,"foo",3); -- cgit v1.2.3