[PATCH v2 12/14] tests/test-logging: Fix test for -dfilter 0..0xffffffffffffffff

Markus Armbruster posted 14 patches 5 years, 9 months ago
Maintainers: Juan Quintela <quintela@redhat.com>, Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Richard Henderson <rth@twiddle.net>, Michael Roth <mdroth@linux.vnet.ibm.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Peter Maydell <peter.maydell@linaro.org>, Max Reitz <mreitz@redhat.com>, Stefano Stabellini <sstabellini@kernel.org>, Paul Durrant <paul@xen.org>, Anthony Perard <anthony.perard@citrix.com>, Jason Wang <jasowang@redhat.com>, Hailiang Zhang <zhang.zhanghailiang@huawei.com>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH v2 12/14] tests/test-logging: Fix test for -dfilter 0..0xffffffffffffffff
Posted by Markus Armbruster 5 years, 9 months ago
Fixes: 58e19e6e7914354242a67442d0006f9e31684d1a
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/test-logging.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test-logging.c b/tests/test-logging.c
index 6387e4933f..8580b82420 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -73,10 +73,10 @@ static void test_parse_range(void)
     g_assert(qemu_log_in_addr_range(UINT64_MAX));
     g_assert_false(qemu_log_in_addr_range(UINT64_MAX - 1));
 
-    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &err);
+    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &error_abort);
     g_assert(qemu_log_in_addr_range(0));
     g_assert(qemu_log_in_addr_range(UINT64_MAX));
- 
+
     qemu_set_dfilter_ranges("2..1", &err);
     error_free_or_abort(&err);
 
-- 
2.21.1


Re: [PATCH v2 12/14] tests/test-logging: Fix test for -dfilter 0..0xffffffffffffffff
Posted by Philippe Mathieu-Daudé 5 years, 9 months ago
Hi Markus,

On 4/22/20 3:07 PM, Markus Armbruster wrote:
> Fixes: 58e19e6e7914354242a67442d0006f9e31684d1a
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   tests/test-logging.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/test-logging.c b/tests/test-logging.c
> index 6387e4933f..8580b82420 100644
> --- a/tests/test-logging.c
> +++ b/tests/test-logging.c
> @@ -73,10 +73,10 @@ static void test_parse_range(void)
>       g_assert(qemu_log_in_addr_range(UINT64_MAX));
>       g_assert_false(qemu_log_in_addr_range(UINT64_MAX - 1));
>   
> -    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &err);
> +    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &error_abort);

Why sometime use this form, ...

>       g_assert(qemu_log_in_addr_range(0));
>       g_assert(qemu_log_in_addr_range(UINT64_MAX));
> -
> +
>       qemu_set_dfilter_ranges("2..1", &err);
>       error_free_or_abort(&err);

... and then this other form?

>   
> 


Re: [PATCH v2 12/14] tests/test-logging: Fix test for -dfilter 0..0xffffffffffffffff
Posted by Eric Blake 5 years, 9 months ago
On 4/22/20 8:35 AM, Philippe Mathieu-Daudé wrote:
> Hi Markus,
> 
> On 4/22/20 3:07 PM, Markus Armbruster wrote:
>> Fixes: 58e19e6e7914354242a67442d0006f9e31684d1a
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   tests/test-logging.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/test-logging.c b/tests/test-logging.c
>> index 6387e4933f..8580b82420 100644
>> --- a/tests/test-logging.c
>> +++ b/tests/test-logging.c
>> @@ -73,10 +73,10 @@ static void test_parse_range(void)
>>       g_assert(qemu_log_in_addr_range(UINT64_MAX));
>>       g_assert_false(qemu_log_in_addr_range(UINT64_MAX - 1));
>> -    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &err);
>> +    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &error_abort);
> 
> Why sometime use this form, ...

This call must not produce an error (if it does, the test aborts, 
proving we had a bug).

> 
>>       g_assert(qemu_log_in_addr_range(0));
>>       g_assert(qemu_log_in_addr_range(UINT64_MAX));
>> -
>> +
>>       qemu_set_dfilter_ranges("2..1", &err);
>>       error_free_or_abort(&err);
> 
> ... and then this other form?

This call must produce an error (if we do not diagnose the caller's 
error, our code is buggy, and the test must fail)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


Re: [PATCH v2 12/14] tests/test-logging: Fix test for -dfilter 0..0xffffffffffffffff
Posted by Philippe Mathieu-Daudé 5 years, 9 months ago
On 4/22/20 4:49 PM, Eric Blake wrote:
> On 4/22/20 8:35 AM, Philippe Mathieu-Daudé wrote:
>> Hi Markus,
>>
>> On 4/22/20 3:07 PM, Markus Armbruster wrote:
>>> Fixes: 58e19e6e7914354242a67442d0006f9e31684d1a
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>   tests/test-logging.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/test-logging.c b/tests/test-logging.c
>>> index 6387e4933f..8580b82420 100644
>>> --- a/tests/test-logging.c
>>> +++ b/tests/test-logging.c
>>> @@ -73,10 +73,10 @@ static void test_parse_range(void)
>>>       g_assert(qemu_log_in_addr_range(UINT64_MAX));
>>>       g_assert_false(qemu_log_in_addr_range(UINT64_MAX - 1));
>>> -    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &err);
>>> +    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &error_abort);
>>
>> Why sometime use this form, ...
> 
> This call must not produce an error (if it does, the test aborts, 
> proving we had a bug).
> 
>>
>>>       g_assert(qemu_log_in_addr_range(0));
>>>       g_assert(qemu_log_in_addr_range(UINT64_MAX));
>>> -
>>> +
>>>       qemu_set_dfilter_ranges("2..1", &err);
>>>       error_free_or_abort(&err);
>>
>> ... and then this other form?
> 
> This call must produce an error (if we do not diagnose the caller's 
> error, our code is buggy, and the test must fail)

Ah OK it makes sense, thanks!


Re: [PATCH v2 12/14] tests/test-logging: Fix test for -dfilter 0..0xffffffffffffffff
Posted by Markus Armbruster 5 years, 9 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Hi Markus,
>
> On 4/22/20 3:07 PM, Markus Armbruster wrote:
>> Fixes: 58e19e6e7914354242a67442d0006f9e31684d1a
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   tests/test-logging.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/test-logging.c b/tests/test-logging.c
>> index 6387e4933f..8580b82420 100644
>> --- a/tests/test-logging.c
>> +++ b/tests/test-logging.c
>> @@ -73,10 +73,10 @@ static void test_parse_range(void)
>>       g_assert(qemu_log_in_addr_range(UINT64_MAX));
>>       g_assert_false(qemu_log_in_addr_range(UINT64_MAX - 1));
>>   -    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &err);
>> +    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &error_abort);
>
> Why sometime use this form, ...
>
>>       g_assert(qemu_log_in_addr_range(0));
>>       g_assert(qemu_log_in_addr_range(UINT64_MAX));
>> -
>> +
>>       qemu_set_dfilter_ranges("2..1", &err);
>>       error_free_or_abort(&err);
>
> ... and then this other form?

The first form crashes when the function sets an error.

The second from crashes when the function doesn't set an error, or else
frees the error.

All clear?


Re: [PATCH v2 12/14] tests/test-logging: Fix test for -dfilter 0..0xffffffffffffffff
Posted by Philippe Mathieu-Daudé 5 years, 9 months ago
On 4/22/20 5:19 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> Hi Markus,
>>
>> On 4/22/20 3:07 PM, Markus Armbruster wrote:
>>> Fixes: 58e19e6e7914354242a67442d0006f9e31684d1a
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>    tests/test-logging.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/test-logging.c b/tests/test-logging.c
>>> index 6387e4933f..8580b82420 100644
>>> --- a/tests/test-logging.c
>>> +++ b/tests/test-logging.c
>>> @@ -73,10 +73,10 @@ static void test_parse_range(void)
>>>        g_assert(qemu_log_in_addr_range(UINT64_MAX));
>>>        g_assert_false(qemu_log_in_addr_range(UINT64_MAX - 1));
>>>    -    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &err);
>>> +    qemu_set_dfilter_ranges("0..0xffffffffffffffff", &error_abort);
>>
>> Why sometime use this form, ...
>>
>>>        g_assert(qemu_log_in_addr_range(0));
>>>        g_assert(qemu_log_in_addr_range(UINT64_MAX));
>>> -
>>> +
>>>        qemu_set_dfilter_ranges("2..1", &err);
>>>        error_free_or_abort(&err);
>>
>> ... and then this other form?
> 
> The first form crashes when the function sets an error.
> 
> The second from crashes when the function doesn't set an error, or else
> frees the error.
> 
> All clear?

Yes =) It is even documented!

  * Assert that an expected error occurred, but clean it up without
  * reporting it (primarily useful in testsuites):
  *     error_free_or_abort(&err);

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>