[PATCH for-8.2] test-resv-mem: Fix CID 1523911

[PATCH for-8.2] test-resv-mem: Fix CID 1523911

[Eric Auger]

Coverity complains about passing "&expected" to "run_range_inverse_array",
which dereferences null "expected". I guess the problem is that the
compare_ranges() loop dereferences 'e' without testing it. However the
loop condition is based on 'ranges' which is garanteed to have
the same length as 'expected' given the g_assert_cmpint() just
before the loop. So the code looks safe to me.

Nevertheless adding a test on expected before the loop to get rid of the
warning.

Fixes: CID 1523901
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reported-by: Coverity (CID 1523901)

---

Hope this fixes the Coverity warning as I cannot test.
---
 tests/unit/test-resv-mem.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/unit/test-resv-mem.c b/tests/unit/test-resv-mem.c
index 5963274e2c..cd8f7318cc 100644
--- a/tests/unit/test-resv-mem.c
+++ b/tests/unit/test-resv-mem.c
@@ -44,6 +44,10 @@ static void compare_ranges(const char *prefix, GList *ranges,
     print_ranges("out", ranges);
     print_ranges("expected", expected);
 #endif
+    if (!expected) {
+        g_assert_true(!ranges);
+        return;
+    }
     g_assert_cmpint(g_list_length(ranges), ==, g_list_length(expected));
     for (l = ranges, e = expected; l ; l = l->next, e = e->next) {
         Range *r = (Range *)l->data;
-- 
2.41.0

Re: [PATCH for-8.2] test-resv-mem: Fix CID 1523911

[Thomas Huth]

On 10/11/2023 09.36, Eric Auger wrote:
> Coverity complains about passing "&expected" to "run_range_inverse_array",
> which dereferences null "expected". I guess the problem is that the
> compare_ranges() loop dereferences 'e' without testing it. However the
> loop condition is based on 'ranges' which is garanteed to have
> the same length as 'expected' given the g_assert_cmpint() just
> before the loop. So the code looks safe to me.
> 
> Nevertheless adding a test on expected before the loop to get rid of the
> warning.
> 
> Fixes: CID 1523901
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reported-by: Coverity (CID 1523901)
> 
> ---
> 
> Hope this fixes the Coverity warning as I cannot test.
> ---
>   tests/unit/test-resv-mem.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/tests/unit/test-resv-mem.c b/tests/unit/test-resv-mem.c
> index 5963274e2c..cd8f7318cc 100644
> --- a/tests/unit/test-resv-mem.c
> +++ b/tests/unit/test-resv-mem.c
> @@ -44,6 +44,10 @@ static void compare_ranges(const char *prefix, GList *ranges,
>       print_ranges("out", ranges);
>       print_ranges("expected", expected);
>   #endif
> +    if (!expected) {
> +        g_assert_true(!ranges);
> +        return;
> +    }
>       g_assert_cmpint(g_list_length(ranges), ==, g_list_length(expected));
>       for (l = ranges, e = expected; l ; l = l->next, e = e->next) {
>           Range *r = (Range *)l->data;

Reviewed-by: Thomas Huth <thuth@redhat.com>

I'll queue it (unless somebody else wants to take this?).

Re: [PATCH for-8.2] test-resv-mem: Fix CID 1523911

[Cédric Le Goater]

On 11/13/23 08:21, Thomas Huth wrote:
> On 10/11/2023 09.36, Eric Auger wrote:
>> Coverity complains about passing "&expected" to "run_range_inverse_array",
>> which dereferences null "expected". I guess the problem is that the
>> compare_ranges() loop dereferences 'e' without testing it. However the
>> loop condition is based on 'ranges' which is garanteed to have
>> the same length as 'expected' given the g_assert_cmpint() just
>> before the loop. So the code looks safe to me.
>>
>> Nevertheless adding a test on expected before the loop to get rid of the
>> warning.
>>
>> Fixes: CID 1523901
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reported-by: Coverity (CID 1523901)
>>
>> ---
>>
>> Hope this fixes the Coverity warning as I cannot test.
>> ---
>>   tests/unit/test-resv-mem.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/tests/unit/test-resv-mem.c b/tests/unit/test-resv-mem.c
>> index 5963274e2c..cd8f7318cc 100644
>> --- a/tests/unit/test-resv-mem.c
>> +++ b/tests/unit/test-resv-mem.c
>> @@ -44,6 +44,10 @@ static void compare_ranges(const char *prefix, GList *ranges,
>>       print_ranges("out", ranges);
>>       print_ranges("expected", expected);
>>   #endif
>> +    if (!expected) {
>> +        g_assert_true(!ranges);
>> +        return;
>> +    }
>>       g_assert_cmpint(g_list_length(ranges), ==, g_list_length(expected));
>>       for (l = ranges, e = expected; l ; l = l->next, e = e->next) {
>>           Range *r = (Range *)l->data;
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
> I'll queue it (unless somebody else wants to take this?).

There is also another fix for the same series :

   https://lore.kernel.org/qemu-devel/20231109170715.259520-1-eric.auger@redhat.com/

I was waiting to have a little more for VFIO to queue both but
they could go through your queue also.

Thanks,

C.

Re: [PATCH for-8.2] test-resv-mem: Fix CID 1523911

[Thomas Huth]

On 13/11/2023 08.56, Cédric Le Goater wrote:
> On 11/13/23 08:21, Thomas Huth wrote:
>> On 10/11/2023 09.36, Eric Auger wrote:
>>> Coverity complains about passing "&expected" to "run_range_inverse_array",
>>> which dereferences null "expected". I guess the problem is that the
>>> compare_ranges() loop dereferences 'e' without testing it. However the
>>> loop condition is based on 'ranges' which is garanteed to have
>>> the same length as 'expected' given the g_assert_cmpint() just
>>> before the loop. So the code looks safe to me.
>>>
>>> Nevertheless adding a test on expected before the loop to get rid of the
>>> warning.
>>>
>>> Fixes: CID 1523901
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> Reported-by: Coverity (CID 1523901)
>>>
>>> ---
>>>
>>> Hope this fixes the Coverity warning as I cannot test.
>>> ---
>>>   tests/unit/test-resv-mem.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/tests/unit/test-resv-mem.c b/tests/unit/test-resv-mem.c
>>> index 5963274e2c..cd8f7318cc 100644
>>> --- a/tests/unit/test-resv-mem.c
>>> +++ b/tests/unit/test-resv-mem.c
>>> @@ -44,6 +44,10 @@ static void compare_ranges(const char *prefix, GList 
>>> *ranges,
>>>       print_ranges("out", ranges);
>>>       print_ranges("expected", expected);
>>>   #endif
>>> +    if (!expected) {
>>> +        g_assert_true(!ranges);
>>> +        return;
>>> +    }
>>>       g_assert_cmpint(g_list_length(ranges), ==, g_list_length(expected));
>>>       for (l = ranges, e = expected; l ; l = l->next, e = e->next) {
>>>           Range *r = (Range *)l->data;
>>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>
>> I'll queue it (unless somebody else wants to take this?).
> 
> There is also another fix for the same series :
> 
>    
> https://lore.kernel.org/qemu-devel/20231109170715.259520-1-eric.auger@redhat.com/
> 
> I was waiting to have a little more for VFIO to queue both but
> they could go through your queue also.

I'm just looking at patches that affect tests/ ... the other fix is not 
related to that, so I won't pick up that one.

  Thomas