qapi: Rewrite string-input-visitor's integer and list parsing
v3 cover qapi: Rewrite string-input-visitor's integer and list parsing
Rewrite string-input-visitor to be (hopefully) less ugly. Support int and uint lists (including ranges, but not implemented via type "Range").
Virtual walks are now supported and more errors are cought (and some bugs fixed). Fix and extend the tests. Parsing of uint64_t is now properly supported.
Importantly, when parsing a list we now return the list and not an ordered set (we are not an ordered set parser after all). Whoever needs that can add it on top. As far as I can see, current code can deal with it but I'll have to look at the details.
While at it, introduce and use qemu_strtod() and qemu_strtod_finite(). However don't convert monitor.c as that seems to be too broken for me (it ignores both conversion errors and range errors, and there is an actual unit test in which conversion is expected to fail).
v2 -> v3:
- "cutils: Add qemu_strtod() and qemu_strtod_finite()" -- Avoid UTF-8 characters
-- Proper -ERANGE handling
- Minor comment/subject/description changes
- Added RBs
v1 -> v2:
- "cutils: Add qemu_strtod() and qemu_strtod_finite()" -- Add a proper documentation
-- Make -EINVAL perform "no conversion" in qemu_strtod_finite().
- "cutils: Fix qemu_strtosz() & friends to reject non-finite sizes" -- New subject/description
-- Added test case
-- Forward -ERANGE
- "qapi: Fix string-input-visitor to reject NaN and infinities" -- New subject/description
-- Added test case
- "test-string-input-visitor: Add more tests"
-- Add even more tests (descending range, overflow, range limit)
- "qapi: Rewrite string-input-visitor"
-- Accept ranges with one element (e.g. 1-1)
-- Bail out on big ranges (just like before)
-- Minor style fixed + description updates
- "test-string-input-visitor: split off uint64 list tests" -- ALso copy + adapt newly added tests for int64 lists
RFC -> v1:
- Too much to name it all, just some highlights
- Pull some tests before the rework
- Introduce qemu_strtod_finite() and use it in more places
- More tests
- "qapi: rewrite string-input-visitor"
-- programming error -> assert
-- g_assert() -> assert(), g_assert_not_reached() -> abort() -- Accept single-element ranges "1-1"
-- Split parsing and returning the next range element -- Minor style fixes
David Hildenbrand (9):
cutils: Add qemu_strtod() and qemu_strtod_finite()
cutils: Fix qemu_strtosz() & friends to reject non-finite sizes
qapi: Fix string-input-visitor to reject NaN and infinities
qapi: Use qemu_strtod_finite() in qobject-input-visitor
test-string-input-visitor: Add more tests
qapi: Rewrite string-input-visitor's integer and list parsing
test-string-input-visitor: Use virtual walk
test-string-input-visitor: Split off uint64 list tests
test-string-input-visitor: Add range overflow tests
include/qapi/string-input-visitor.h | 4 +- include/qemu/cutils.h | 8 +- monitor.c | 2 +- qapi/qobject-input-visitor.c | 9 +- qapi/string-input-visitor.c | 413 ++++++++++++++++------------ tests/test-cutils.c | 24 +- tests/test-string-input-visitor.c | 223 ++++++++++++--- util/cutils.c | 83 +++++- 8 files changed, 517 insertions(+), 249 deletions(-)
Signature · 1 line
2.17.2
PATCH v3 1/9 cutils: Add qemu_strtod() and qemu_strtod_finite()
Let's provide a wrapper for strtod().
--- include/qemu/cutils.h | 2 ++ util/cutils.c | 65 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+)
include/qemu/cutils.h +2 -0
@@ -146,6 +146,8 @@ int qemu_strtoi64(const char *nptr, const char **endptr, int base, int64_t *result);int qemu_strtou64 (const char *nptr,const char **endptr,int base,uint64_t *result); +int qemu_strtod (const char *nptr,const char **endptr,double *result); +int qemu_strtod_finite (const char *nptr,const char **endptr,double *result);int parse_uint (const char *s,unsigned long long *value,char **endptr,int base);
util/cutils.c +65 -0
71 lines not on this page · raw message
Signature · 1 line
2.17.2
Re: [Qemu-devel] [PATCH v3 for-4.0 1/9] cutils: Add qemu_strtod() and qemu_strtod_finite()
Quoted 8 lines · David Hildenbrand
Let's provide a wrapper for
strtod().Reviewed-by Markus Armbruster <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- include/qemu/cutils.h | 2 ++ util/cutils.c | 65 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+)
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
PATCH v3 2/9 cutils: Fix qemu_strtosz() & friends to reject non-finite sizes
In reply to David Hildenbrandqemu_strtosz() & friends reject NaNs, but happily accept infinities. They shouldn't. Fix that.
The fix makes use of qemu_strtod_finite(). To avoid ugly casts, change the @end parameter of qemu_strtosz() & friends from char ** to const char **.
Also, add two test cases, testing that "inf" and "NaN" are properly rejected. While at it, also fixup the function documentation.
--- include/qemu/cutils.h | 6 +++--- monitor.c | 2 +- tests/test-cutils.c | 24 +++++++++++++++++------- util/cutils.c | 18 ++++++++---------- 4 files changed, 29 insertions(+), 21 deletions(-)
include/qemu/cutils.h +3 -3
@@ -153,9 +153,9 @@ int parse_uint(const char *s, unsigned long long *value, char **endptr, int base);int parse_uint_full (const char *s,unsigned long long *value,int base); -int qemu_strtosz (const char *nptr,char **end,uint64_t *result); -int qemu_strtosz_MiB (const char *nptr,char **end,uint64_t *result); -int qemu_strtosz_metric (const char *nptr,char **end,uint64_t *result); +int qemu_strtosz (const char *nptr,const char **end,uint64_t *result); +int qemu_strtosz_MiB (const char *nptr,const char **end,uint64_t *result); +int qemu_strtosz_metric (const char *nptr,const char **end,uint64_t *result);/* used to print char* safely */ #define STR_OR_NULL (str) ((str) ? (str) : "null")
monitor.c +1 -1
@@ -3231,7 +3231,7 @@ static QDict *monitor_parse_arguments(Monitor *mon, {int ret;uint64_t val; -char *end; +const char *end;while (qemu_isspace (*p)) { p++;
tests/test-cutils.c +17 -7
@@ -1950,7 +1950,7 @@ static void test_qemu_strtou64_full_max(void)static void test_qemu_strtosz_simple (void ) {const char *str; -char *endptr =NULL ; +const char *endptr;int err;uint64_t res =0xbaadf00d ;
@@ -2017,7 +2017,7 @@ static void test_qemu_strtosz_units(void)const char *p ="1P" ;const char *e ="1E" ;int err; -char *endptr =NULL ; +const char *endptr;uint64_t res =0xbaadf00d ;/* default is M */
@@ -2066,7 +2066,7 @@ static void test_qemu_strtosz_float(void) {const char *str ="12.345M" ;int err; -char *endptr =NULL ; +const char *endptr;uint64_t res =0xbaadf00d ; err =qemu_strtosz (str, &endptr, &res);
@@ -2078,7 +2078,7 @@ static void test_qemu_strtosz_float(void)static void test_qemu_strtosz_invalid (void ) {const char *str; -char *endptr =NULL ; +const char *endptr;int err;uint64_t res =0xbaadf00d ;
@@ -2096,12 +2096,22 @@ static void test_qemu_strtosz_invalid(void) err =qemu_strtosz (str, &endptr, &res); g_assert_cmpint(err, ==, -EINVAL);g_assert (endptr == str); + + str ="inf" ; + err =qemu_strtosz (str, &endptr, &res); + g_assert_cmpint(err, ==, -EINVAL); +g_assert (endptr == str); + + str ="NaN" ; + err =qemu_strtosz (str, &endptr, &res); + g_assert_cmpint(err, ==, -EINVAL); +g_assert (endptr == str); }static void test_qemu_strtosz_trailing (void ) {const char *str; -char *endptr =NULL ; +const char *endptr;int err;uint64_t res =0xbaadf00d ;
@@ -2126,7 +2136,7 @@ static void test_qemu_strtosz_trailing(void)static void test_qemu_strtosz_erange (void ) {const char *str; -char *endptr =NULL ; +const char *endptr;int err;uint64_t res =0xbaadf00d ;
@@ -2160,7 +2170,7 @@ static void test_qemu_strtosz_metric(void) {const char *str ="12345k" ;int err; -char *endptr =NULL ; +const char *endptr;uint64_t res =0xbaadf00d ; err =qemu_strtosz_metric (str, &endptr, &res);
util/cutils.c +8 -10
@@ -203,23 +203,21 @@ static int64_t suffix_mul(char suffix, int64_t unit)/* * Convert string to bytes, allowing either B/b for bytes, K/k for KB, * M/m for MB, G/g for GB or T/t for TB. End pointer will be returned -* in *end, if not NULL. Return -ERANGE on overflow, Return -EINVAL on +* in *end, if not NULL. Return -ERANGE on overflow, and -EINVAL on * other error. */ -static int do_strtosz (const char *nptr,char **end, +static int do_strtosz (const char *nptr,const char **end,const char default_suffix,int64_t unit,uint64_t *result) {int retval; -char *endptr; +const char *endptr;unsigned char c;int mul_required =0 ;double val, mul, integral, fraction; - errno =0 ; - val =strtod (nptr, &endptr); -if (isnan (val) || endptr == nptr || errno !=0 ) { - retval = -EINVAL ; + retval =qemu_strtod_finite (nptr, &endptr, &val); +if (retval) { goto out; } fraction =modf (val, &integral);
@@ -259,17 +257,17 @@ out:return retval; } -int qemu_strtosz (const char *nptr,char **end,uint64_t *result) +int qemu_strtosz (const char *nptr,const char **end,uint64_t *result) {return do_strtosz (nptr, end,'B' ,1024 , result); } -int qemu_strtosz_MiB (const char *nptr,char **end,uint64_t *result) +int qemu_strtosz_MiB (const char *nptr,const char **end,uint64_t *result) {return do_strtosz (nptr, end,'M' ,1024 , result); } -int qemu_strtosz_metric (const char *nptr,char **end,uint64_t *result) +int qemu_strtosz_metric (const char *nptr,const char **end,uint64_t *result) {return do_strtosz (nptr, end,'B' ,1000 , result); }
Signature · 1 line
2.17.2
PATCH v3 3/9 qapi: Fix string-input-visitor to reject NaN and infinities
In reply to David HildenbrandThe string-input-visitor happily accepts NaN and infinities when parsing numbers (doubles). They shouldn't. Fix that.
Also, add two test cases, testing if "NaN" and "inf" is properly rejected.
--- qapi/string-input-visitor.c | 6 ++---- tests/test-string-input-visitor.c | 13 +++++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-)
qapi/string-input-visitor.c +2 -4
@@ -20,6 +20,7 @@#include "qemu/option.h" #include "qemu/queue.h" #include "qemu/range.h" +#include "qemu/cutils.h" struct StringInputVisitor
@@ -313,12 +314,9 @@ static void parse_type_number(Visitor *v, const char *name, double *obj, Error **errp) {StringInputVisitor *siv =to_siv (v); -char *endp = (char *) siv->string;double val; - errno =0 ; - val =strtod (siv->string, &endp); -if (errno || endp == siv->string || *endp) { +if (qemu_strtod_finite (siv->string,NULL , &val)) {error_setg (errp,QERR_INVALID_PARAMETER_TYPE , name ? name :"null" ,"number" );return ;
tests/test-string-input-visitor.c +13 -0
@@ -252,6 +252,19 @@ static void test_visitor_in_number(TestInputVisitorData *data,visit_type_number (v,NULL , &res, &err);g_assert (!err); g_assert_cmpfloat(res, ==, value); + +/* NaN and infinity has to be rejected */ + + v =visitor_input_test_init (data,"NaN" ); + +visit_type_number (v,NULL , &res, &err); +error_free_or_abort (&err); + + v =visitor_input_test_init (data,"inf" ); + +visit_type_number (v,NULL , &res, &err); +error_free_or_abort (&err); + } static void test_visitor_in_string(TestInputVisitorData *data,
Signature · 1 line
2.17.2
PATCH v3 4/9 qapi: Use qemu_strtod_finite() in qobject-input-visitor
In reply to David HildenbrandLet's use the new function. Just as current behavior, we have to consume the whole string (now it's just way clearer what's going on).
--- qapi/qobject-input-visitor.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
qapi/qobject-input-visitor.c +5 -4
@@ -562,19 +562,20 @@ static void qobject_input_type_number_keyval(Visitor *v, const char *name, {QObjectInputVisitor *qiv =to_qiv (v);const char *str =qobject_input_get_keyval (qiv, name, errp); -char *endp; +double val;if (!str) {return ; } - errno =0 ; - *obj =strtod (str, &endp); -if (errno || endp == str || *endp || !isfinite (*obj)) { +if (qemu_strtod_finite (str,NULL , &val)) {/* TODO report -ERANGE more nicely */ error_setg (errp,QERR_INVALID_PARAMETER_TYPE ,full_name (qiv, name),"number" ); +return ; } + + *obj = val; } static void qobject_input_type_any(Visitor *v, const char *name, QObject **obj,
Signature · 1 line
2.17.2
PATCH v3 5/9 test-string-input-visitor: Add more tests
In reply to David HildenbrandTest that very big/small values are not accepted and that ranges with only one element work. Also test that ranges are ascending and cannot have more than 65536 elements.
Rename expect4 to expect5, as we will be moving that to a separate ulist test after the rework.
--- tests/test-string-input-visitor.c | 41 +++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-)
tests/test-string-input-visitor.c +39 -2
@@ -121,7 +121,8 @@ static void test_visitor_in_intList(TestInputVisitorData *data,int64_t expect1[] = {0 ,1 ,2 ,3 ,4 ,5 ,6 ,7 ,8 ,9 ,20 };int64_t expect2[] = {32767 ,-32768 ,-32767 };int64_t expect3[] = {INT64_MAX ,INT64_MIN }; -uint64_t expect4[] = {UINT64_MAX }; +int64_t expect4[] = {1 }; +uint64_t expect5[] = {UINT64_MAX };Error *err =NULL ;int64List *res =NULL ;int64List *tail;
@@ -140,8 +141,44 @@ static void test_visitor_in_intList(TestInputVisitorData *data, "-9223372036854775808,9223372036854775807");check_ilist (v, expect3,ARRAY_SIZE (expect3)); + v =visitor_input_test_init (data,"1-1" ); +check_ilist (v, expect4,ARRAY_SIZE (expect4)); + v =visitor_input_test_init (data,"18446744073709551615" ); -check_ulist (v, expect4,ARRAY_SIZE (expect4)); +check_ulist (v, expect5,ARRAY_SIZE (expect5)); + +/* Value too large */ + + v =visitor_input_test_init (data,"9223372036854775808" ); +visit_type_int64List (v,NULL , &res, &err); +error_free_or_abort (&err); +g_assert (!res); + +/* Value too small */ + + v =visitor_input_test_init (data,"-9223372036854775809" ); +visit_type_int64List (v,NULL , &res, &err); +error_free_or_abort (&err); +g_assert (!res); + +/* Range not ascending */ + + v =visitor_input_test_init (data,"3-1" ); +visit_type_int64List (v,NULL , &res, &err); +error_free_or_abort (&err); +g_assert (!res); + + v =visitor_input_test_init (data,"9223372036854775807-0" ); +visit_type_int64List (v,NULL , &res, &err); +error_free_or_abort (&err); +g_assert (!res); + +/* Range too big (65536 is the limit against DOS attacks) */ + + v =visitor_input_test_init (data,"0-65536" ); +visit_type_int64List (v,NULL , &res, &err); +error_free_or_abort (&err); +g_assert (!res);/* Empty list */
Signature · 1 line
2.17.2
PATCH v3 6/9 qapi: Rewrite string-input-visitor's integer and list parsing
In reply to David HildenbrandThe input visitor has some problems right now, especially
- unsigned type "Range" is used to process signed ranges, resulting in inconsistent behavior and ugly/magical code
- uint64_t are parsed like int64_t, so big uint64_t values are not supported and error messages are misleading
- lists/ranges of int64_t are accepted although no list is parsed and we should rather report an error
- lists/ranges are preparsed using int64_t, making it hard to implement uint64_t values or uint64_t lists
- types that don't support lists don't bail out
- visiting beyond the end of a list is not handled properly
- we don't actually parse lists, we parse *sets*: members are sorted, and duplicates eliminated
So let's rewrite it by getting rid of usage of the type "Range" and properly supporting lists of int64_t and uint64_t (including ranges of both types), fixing the above mentioned issues.
Lists of other types are not supported and will properly report an error. Virtual walks are now supported.
Tests have to be fixed up:
- Two BUGs were hardcoded that are fixed now
- The string-input-visitor now actually returns a parsed list and not an ordered set.
Please note that no users/callers have to be fixed up. Candidates using visit_type_uint16List() and friends are:
- backends/hostmem.c:host_memory_backend_set_host_nodes()
-- Code can deal with duplicates/unsorted lists
- numa.c::query_memdev()
-- via object_property_get_uint16List(), the list will still be sorted
and without duplicates (via host_memory_backend_get_host_nodes())
- qapi-visit.c::visit_type_Memdev_members()
- qapi-visit.c::visit_type_NumaNodeOptions_members()
- qapi-visit.c::visit_type_RockerOfDpaGroup_members
- qapi-visit.c::visit_type_RxFilterInfo_members()
-- Not used with string-input-visitor.
--- include/qapi/string-input-visitor.h | 4 +- qapi/string-input-visitor.c | 405 ++++++++++++++++------------ tests/test-string-input-visitor.c | 18 +- 3 files changed, 234 insertions(+), 193 deletions(-)
include/qapi/string-input-visitor.h +2 -2
@@ -19,8 +19,8 @@ typedef struct StringInputVisitor StringInputVisitor;/* * The string input visitor does not implement support for visiting -* QAPI structs, alternates, null, or arbitrary QTypes. It also -* requires a non-null list argument to visit_start_list(). +* QAPI structs, alternates, null, or arbitrary QTypes. Only flat lists +* of integers (except type "size") are supported. */ Visitor *string_input_visitor_new (const char *str);
qapi/string-input-visitor.c +228 -177
529 lines not on this page · raw message
tests/test-string-input-visitor.c +4 -14
@@ -92,16 +92,6 @@ static void check_ulist(Visitor *v, uint64_t *expected, size_t n)uint64List *tail;int i; -/* BUG: unsigned numbers above INT64_MAX don't work */ -for (i =0 ; i < n; i++) { -if (expected[i] >INT64_MAX ) { -Error *err =NULL ; -visit_type_uint64List (v,NULL , &res, &err); -error_free_or_abort (&err); -return ; - } - } -visit_type_uint64List (v,NULL , &res, &error_abort); tail = res;for (i =0 ; i < n; i++) {
@@ -117,10 +107,10 @@ static void check_ulist(Visitor *v, uint64_t *expected, size_t n)static void test_visitor_in_intList (TestInputVisitorData *data,const void *unused) { -/* Note: the visitor *sorts* ranges *unsigned* */ -int64_t expect1[] = {0 ,1 ,2 ,3 ,4 ,5 ,6 ,7 ,8 ,9 ,20 }; +int64_t expect1[] = {1 ,2 ,0 ,2 ,3 ,4 ,20 ,5 ,6 ,7 , +8 ,9 ,1 ,2 ,3 ,4 ,5 ,6 ,7 ,8 };int64_t expect2[] = {32767 ,-32768 ,-32767 }; -int64_t expect3[] = {INT64_MAX ,INT64_MIN }; +int64_t expect3[] = {INT64_MIN ,INT64_MAX };int64_t expect4[] = {1 };uint64_t expect5[] = {UINT64_MAX };Error *err =NULL ;
@@ -226,7 +216,7 @@ static void test_visitor_in_intList(TestInputVisitorData *data,visit_type_int64 (v,NULL , &tail->value, &err); g_assert_cmpint(tail->value, ==, 0);visit_type_int64 (v,NULL , &val, &err); - g_assert_cmpint(val, ==, 1); /* BUG */ +error_free_or_abort (&err);visit_check_list (v, &error_abort);visit_end_list (v, (void **)&res);
Signature · 1 line
2.17.2
PATCH v3 7/9 test-string-input-visitor: Use virtual walk
In reply to David HildenbrandWe now support virtual walks, so use that instead.
--- tests/test-string-input-visitor.c | 36 +++++++++++-------------------- 1 file changed, 12 insertions(+), 24 deletions(-)
tests/test-string-input-visitor.c +12 -24
@@ -115,7 +115,6 @@ static void test_visitor_in_intList(TestInputVisitorData *data,uint64_t expect5[] = {UINT64_MAX };Error *err =NULL ;int64List *res =NULL ; -int64List *tail;Visitor *v;int64_t val;
@@ -188,39 +187,28 @@ static void test_visitor_in_intList(TestInputVisitorData *data, v =visitor_input_test_init (data,"0,2-3" ); -/* Would be simpler if the visitor genuinely supported virtual walks */ -visit_start_list (v,NULL , (GenericList **)&res,sizeof (*res), - &error_abort); - tail = res; -visit_type_int64 (v,NULL , &tail->value, &error_abort); - g_assert_cmpint(tail->value, ==, 0); - tail = (int64List *)visit_next_list (v, (GenericList *)tail,sizeof (*res)); -g_assert (tail); -visit_type_int64 (v,NULL , &tail->value, &error_abort); - g_assert_cmpint(tail->value, ==, 2); - tail = (int64List *)visit_next_list (v, (GenericList *)tail,sizeof (*res)); -g_assert (tail); +visit_start_list (v,NULL ,NULL ,0 , &error_abort); +visit_type_int64 (v,NULL , &val, &error_abort); + g_assert_cmpint(val, ==, 0); +visit_type_int64 (v,NULL , &val, &error_abort); + g_assert_cmpint(val, ==, 2);visit_check_list (v, &err);error_free_or_abort (&err); -visit_end_list (v, (void **)&res); - -qapi_free_int64List (res); +visit_end_list (v,NULL );/* Visit beyond end of list */ + v =visitor_input_test_init (data,"0" ); -visit_start_list (v,NULL , (GenericList **)&res,sizeof (*res), - &error_abort); - tail = res; -visit_type_int64 (v,NULL , &tail->value, &err); - g_assert_cmpint(tail->value, ==, 0); +visit_start_list (v,NULL ,NULL ,0 , &error_abort); +visit_type_int64 (v,NULL , &val, &err); + g_assert_cmpint(val, ==, 0);visit_type_int64 (v,NULL , &val, &err);error_free_or_abort (&err); -visit_check_list (v, &error_abort); -visit_end_list (v, (void **)&res); -qapi_free_int64List (res); +visit_check_list (v, &error_abort); +visit_end_list (v,NULL ); } static void test_visitor_in_bool(TestInputVisitorData *data,
Signature · 1 line
2.17.2
PATCH v3 8/9 test-string-input-visitor: Split off uint64 list tests
In reply to David HildenbrandBasically copy all int64 list tests but adapt them to work on uint64 instead. The values for very big/very small values have to be adapted.
--- tests/test-string-input-visitor.c | 113 ++++++++++++++++++++++++++++-- 1 file changed, 109 insertions(+), 4 deletions(-)
tests/test-string-input-visitor.c +109 -4
@@ -112,7 +112,6 @@ static void test_visitor_in_intList(TestInputVisitorData *data,int64_t expect2[] = {32767 ,-32768 ,-32767 };int64_t expect3[] = {INT64_MIN ,INT64_MAX };int64_t expect4[] = {1 }; -uint64_t expect5[] = {UINT64_MAX };Error *err =NULL ;int64List *res =NULL ;Visitor *v;
@@ -133,9 +132,6 @@ static void test_visitor_in_intList(TestInputVisitorData *data, v =visitor_input_test_init (data,"1-1" );check_ilist (v, expect4,ARRAY_SIZE (expect4)); - v =visitor_input_test_init (data,"18446744073709551615" ); -check_ulist (v, expect5,ARRAY_SIZE (expect5)); -/* Value too large */ v =visitor_input_test_init (data,"9223372036854775808" );
@@ -211,6 +207,113 @@ static void test_visitor_in_intList(TestInputVisitorData *data,visit_end_list (v,NULL ); } +static void test_visitor_in_uintList (TestInputVisitorData *data, +const void *unused) +{ +uint64_t expect1[] = {1 ,2 ,0 ,2 ,3 ,4 ,20 ,5 ,6 ,7 , +8 ,9 ,1 ,2 ,3 ,4 ,5 ,6 ,7 ,8 }; +uint64_t expect2[] = {32767 ,-32768 ,-32767 }; +uint64_t expect3[] = {INT64_MIN ,INT64_MAX }; +uint64_t expect4[] = {1 }; +uint64_t expect5[] = {UINT64_MAX }; +Error *err =NULL ; +uint64List *res =NULL ; +Visitor *v; +uint64_t val; + +/* Valid lists */ + + v =visitor_input_test_init (data,"1,2,0,2-4,20,5-9,1-8" ); +check_ulist (v, expect1,ARRAY_SIZE (expect1)); + + v =visitor_input_test_init (data,"32767,-32768--32767" ); +check_ulist (v, expect2,ARRAY_SIZE (expect2)); + + v =visitor_input_test_init (data, +"-9223372036854775808,9223372036854775807" ); +check_ulist (v, expect3,ARRAY_SIZE (expect3)); + + v =visitor_input_test_init (data,"1-1" ); +check_ulist (v, expect4,ARRAY_SIZE (expect4)); + + v =visitor_input_test_init (data,"18446744073709551615" ); +check_ulist (v, expect5,ARRAY_SIZE (expect5)); + +/* Value too large */ + + v =visitor_input_test_init (data,"18446744073709551616" ); +visit_type_uint64List (v,NULL , &res, &err); +error_free_or_abort (&err); +g_assert (!res); + +/* Value too small */ + + v =visitor_input_test_init (data,"-18446744073709551616" ); +visit_type_uint64List (v,NULL , &res, &err); +error_free_or_abort (&err); +g_assert (!res); + +/* Range not ascending */ + + v =visitor_input_test_init (data,"3-1" ); +visit_type_uint64List (v,NULL , &res, &err); +error_free_or_abort (&err); +g_assert (!res); + + v =visitor_input_test_init (data,"18446744073709551615-0" ); +visit_type_uint64List (v,NULL , &res, &err); +error_free_or_abort (&err); +g_assert (!res); + +/* Range too big (65536 is the limit against DOS attacks) */ + + v =visitor_input_test_init (data,"0-65536" ); +visit_type_uint64List (v,NULL , &res, &err); +error_free_or_abort (&err); +g_assert (!res); + +/* Empty list */ + + v =visitor_input_test_init (data,"" ); +visit_type_uint64List (v,NULL , &res, &error_abort); +g_assert (!res); + +/* Not a list */ + + v =visitor_input_test_init (data,"not an uint list" ); + +visit_type_uint64List (v,NULL , &res, &err); +error_free_or_abort (&err); +g_assert (!res); + +/* Unvisited list tail */ + + v =visitor_input_test_init (data,"0,2-3" ); + +visit_start_list (v,NULL ,NULL ,0 , &error_abort); +visit_type_uint64 (v,NULL , &val, &error_abort); + g_assert_cmpuint(val, ==, 0); +visit_type_uint64 (v,NULL , &val, &error_abort); + g_assert_cmpuint(val, ==, 2); + +visit_check_list (v, &err); +error_free_or_abort (&err); +visit_end_list (v,NULL ); + +/* Visit beyond end of list */ + + v =visitor_input_test_init (data,"0" ); + +visit_start_list (v,NULL ,NULL ,0 , &error_abort); +visit_type_uint64 (v,NULL , &val, &err); + g_assert_cmpuint(val, ==, 0); +visit_type_uint64 (v,NULL , &val, &err); +error_free_or_abort (&err); + +visit_check_list (v, &error_abort); +visit_end_list (v,NULL ); +} +static void test_visitor_in_bool (TestInputVisitorData *data,const void *unused) {
@@ -384,6 +487,8 @@ int main(int argc, char **argv) &in_visitor_data, test_visitor_in_int);input_visitor_test_add ("/string-visitor/input/intList" , &in_visitor_data, test_visitor_in_intList); +input_visitor_test_add ("/string-visitor/input/uintList" , + &in_visitor_data, test_visitor_in_uintList);input_visitor_test_add ("/string-visitor/input/bool" , &in_visitor_data, test_visitor_in_bool); input_visitor_test_add("/string-visitor/input/number",
Signature · 1 line
2.17.2
PATCH v3 9/9 test-string-input-visitor: Add range overflow tests
In reply to David HildenbrandLet's make sure that the range handling code can properly deal with ranges that end at the biggest possible number.
--- tests/test-string-input-visitor.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
tests/test-string-input-visitor.c +10 -0
@@ -112,6 +112,7 @@ static void test_visitor_in_intList(TestInputVisitorData *data,int64_t expect2[] = {32767 ,-32768 ,-32767 };int64_t expect3[] = {INT64_MIN ,INT64_MAX };int64_t expect4[] = {1 }; +int64_t expect5[] = {INT64_MAX -2 ,INT64_MAX -1 ,INT64_MAX };Error *err =NULL ;int64List *res =NULL ;Visitor *v;
@@ -132,6 +133,10 @@ static void test_visitor_in_intList(TestInputVisitorData *data, v =visitor_input_test_init (data,"1-1" );check_ilist (v, expect4,ARRAY_SIZE (expect4)); + v =visitor_input_test_init (data, +"9223372036854775805-9223372036854775807" ); +check_ilist (v, expect5,ARRAY_SIZE (expect5)); +/* Value too large */ v =visitor_input_test_init (data,"9223372036854775808" );
@@ -216,6 +221,7 @@ static void test_visitor_in_uintList(TestInputVisitorData *data,uint64_t expect3[] = {INT64_MIN ,INT64_MAX };uint64_t expect4[] = {1 };uint64_t expect5[] = {UINT64_MAX }; +uint64_t expect6[] = {UINT64_MAX -2 ,UINT64_MAX -1 ,UINT64_MAX };Error *err =NULL ;uint64List *res =NULL ;Visitor *v;
@@ -239,6 +245,10 @@ static void test_visitor_in_uintList(TestInputVisitorData *data, v =visitor_input_test_init (data,"18446744073709551615" );check_ulist (v, expect5,ARRAY_SIZE (expect5)); + v =visitor_input_test_init (data, +"18446744073709551613-18446744073709551615" ); +check_ulist (v, expect6,ARRAY_SIZE (expect6)); +/* Value too large */ v =visitor_input_test_init (data,"18446744073709551616" );
Signature · 1 line
2.17.2
Markus Armbruster
AppliedQuoted 13 lines · David Hildenbrand
Rewrite string-input-visitor to be (hopefully) less ugly. Support int and uint lists (including ranges, but not implemented via type "Range").
Virtual walks are now supported and more errors are cought (and some bugs fixed). Fix and extend the tests. Parsing of
uint64_tis now properly supported.Importantly, when parsing a list we now return the list and not an ordered set (we are not an ordered set parser after all). Whoever needs that can add it on top. As far as I can see, current code can deal with it but I'll have to look at the details.
Quoted 4 lines
While at it, introduce and use
qemu_strtod()andqemu_strtod_finite(). However don't convertmonitor.cas that seems to be too broken for me (it ignores both conversion errors and range errors, and there is an actual unit test in which conversion is expected to fail).
Queued for 4.0. Thanks a lot, David!