Re: [PATCH 2/2] test-keyval: Tighten test of trailing crap after size
Le 25/11/2019 à 14:38, Markus Armbruster a écrit :
> test_keyval_visit_size() should test for trailing crap after size with
> and without suffix. It does test the latter: "sz2=16Gi" has size
> "16G" followed by crap "i". It fails to test the former "sz1=16E" is
> a syntactically valid size that overflows uint64_t. Replace by
> "sz1=0Z".
>
> Signed-off-by: Markus Armbruster
> ---
> tests/test-keyval.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/test-keyval.c b/tests/test-keyval.c
> index 09b0ae3c68..e331a84149 100644
> --- a/tests/test-keyval.c
> +++ b/tests/test-keyval.c
> @@ -478,7 +478,7 @@ static void test_keyval_visit_size(void)
> visit_free(v);
>
> /* Trailing crap */
> -qdict = keyval_parse("sz1=16E,sz2=16Gi", NULL, _abort);
> +qdict = keyval_parse("sz1=0Z,sz2=16Gi", NULL, _abort);
> v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
> qobject_unref(qdict);
> visit_start_struct(v, NULL, NULL, 0, _abort);
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH 2/2] test-keyval: Tighten test of trailing crap after size
On 11/25/19 9:31 AM, Markus Armbruster wrote:
Eric Blake writes:
On 11/25/19 7:38 AM, Markus Armbruster wrote:
test_keyval_visit_size() should test for trailing crap after size with
and without suffix. It does test the latter: "sz2=16Gi" has size
"16G" followed by crap "i". It fails to test the former "sz1=16E" is
a syntactically valid size that overflows uint64_t. Replace by
"sz1=0Z".
/* Trailing crap */
-qdict = keyval_parse("sz1=16E,sz2=16Gi", NULL, _abort);
+qdict = keyval_parse("sz1=0Z,sz2=16Gi", NULL, _abort);
Does this actually test both failure cases, or does it abort the parse
after the first failure (sz1=0Z) without ever hitting the second half
of the parse (sz2=16Gi)?
Fair question! Short answer: yes, we check both.
Aha - keyval_parse() just sets up the parser, while the check for double
failures is in the test code below.
Clear now?
Yes.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH 2/2] test-keyval: Tighten test of trailing crap after size
Eric Blake writes:
> On 11/25/19 7:38 AM, Markus Armbruster wrote:
>> test_keyval_visit_size() should test for trailing crap after size with
>> and without suffix. It does test the latter: "sz2=16Gi" has size
>> "16G" followed by crap "i". It fails to test the former "sz1=16E" is
>> a syntactically valid size that overflows uint64_t. Replace by
>> "sz1=0Z".
>>
>> Signed-off-by: Markus Armbruster
>> ---
>> tests/test-keyval.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/test-keyval.c b/tests/test-keyval.c
>> index 09b0ae3c68..e331a84149 100644
>> --- a/tests/test-keyval.c
>> +++ b/tests/test-keyval.c
>> @@ -478,7 +478,7 @@ static void test_keyval_visit_size(void)
>> visit_free(v);
>> /* Trailing crap */
>> -qdict = keyval_parse("sz1=16E,sz2=16Gi", NULL, _abort);
>> +qdict = keyval_parse("sz1=0Z,sz2=16Gi", NULL, _abort);
>
> Does this actually test both failure cases, or does it abort the parse
> after the first failure (sz1=0Z) without ever hitting the second half
> of the parse (sz2=16Gi)?
Fair question! Short answer: yes, we check both.
Long answer follows.
/* Trailing crap */
qdict = keyval_parse("time1=89ks,time2=ns", NULL, _abort);
keyval_parse() must succeed: it takes _abort.
v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
qobject_unref(qdict);
Can't fail.
visit_start_struct(v, NULL, NULL, 0, _abort);
Must succeed.
visit_type_size(v, "sz1", , );
error_free_or_abort();
This is where we parse "0Z". Must fail. We continue.
visit_type_size(v, "sz2", , );
error_free_or_abort();
This is where we parse "16Gi". Must fail. We continue.
visit_end_struct(v, NULL);
visit_free(v);
}
Clear now?
Re: [PATCH 2/2] test-keyval: Tighten test of trailing crap after size
On 11/25/19 7:38 AM, Markus Armbruster wrote:
test_keyval_visit_size() should test for trailing crap after size with
and without suffix. It does test the latter: "sz2=16Gi" has size
"16G" followed by crap "i". It fails to test the former "sz1=16E" is
a syntactically valid size that overflows uint64_t. Replace by
"sz1=0Z".
Signed-off-by: Markus Armbruster
---
tests/test-keyval.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index 09b0ae3c68..e331a84149 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -478,7 +478,7 @@ static void test_keyval_visit_size(void)
visit_free(v);
/* Trailing crap */
-qdict = keyval_parse("sz1=16E,sz2=16Gi", NULL, _abort);
+qdict = keyval_parse("sz1=0Z,sz2=16Gi", NULL, _abort);
Does this actually test both failure cases, or does it abort the parse
after the first failure (sz1=0Z) without ever hitting the second half of
the parse (sz2=16Gi)?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[PATCH 2/2] test-keyval: Tighten test of trailing crap after size
test_keyval_visit_size() should test for trailing crap after size with
and without suffix. It does test the latter: "sz2=16Gi" has size
"16G" followed by crap "i". It fails to test the former "sz1=16E" is
a syntactically valid size that overflows uint64_t. Replace by
"sz1=0Z".
Signed-off-by: Markus Armbruster
---
tests/test-keyval.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index 09b0ae3c68..e331a84149 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -478,7 +478,7 @@ static void test_keyval_visit_size(void)
visit_free(v);
/* Trailing crap */
-qdict = keyval_parse("sz1=16E,sz2=16Gi", NULL, _abort);
+qdict = keyval_parse("sz1=0Z,sz2=16Gi", NULL, _abort);
v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
qobject_unref(qdict);
visit_start_struct(v, NULL, NULL, 0, _abort);
--
2.21.0
