ext_unpack_subimage: Ensure piglit_probe_pixel_rgba is checked.

Submitted by Vinson Lee on Aug. 7, 2014, 12:32 a.m.

Details

Message ID 1407371527-8033-1-git-send-email-vlee@freedesktop.org
State New
Headers show

Not browsing as part of any series.

Commit Message

Vinson Lee Aug. 7, 2014, 12:32 a.m.
Fixes Coverity "Unchecked return value" defects.

Signed-off-by: Vinson Lee <vlee@freedesktop.org>
---
 .../spec/ext_unpack_subimage/ext_unpack_subimage.c | 24 +++++++++++-----------
 1 file changed, 12 insertions(+), 12 deletions(-)

Patch hide | download patch | download mbox

diff --git a/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c b/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
index e65f4be..3935d92 100644
--- a/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
+++ b/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
@@ -156,19 +156,19 @@  piglit_display(void)
 			     0, 0, 1, 1);
 
 	if (extension_supported) {
-		pass &= piglit_probe_pixel_rgba(piglit_width / 2,
-						piglit_height / 4,
-						blue);
-		pass &= piglit_probe_pixel_rgba(piglit_width / 2,
-						piglit_height * 3 / 4,
-						cyan);
+		pass = piglit_probe_pixel_rgba(piglit_width / 2,
+					       piglit_height / 4,
+					       blue) && pass;
+		pass = piglit_probe_pixel_rgba(piglit_width / 2,
+					       piglit_height * 3 / 4,
+					       cyan) && pass;
 	} else {
-		pass &= piglit_probe_pixel_rgba(piglit_width / 2,
-						piglit_height / 4,
-						red);
-		pass &= piglit_probe_pixel_rgba(piglit_width / 2,
-						piglit_height * 3 / 4,
-						green);
+		pass = piglit_probe_pixel_rgba(piglit_width / 2,
+					       piglit_height / 4,
+					       red) && pass;
+		pass = piglit_probe_pixel_rgba(piglit_width / 2,
+					       piglit_height * 3 / 4,
+					       green) && pass;
 	}
 
 	return pass ? PIGLIT_PASS : PIGLIT_FAIL;

Comments

On Wed, Aug 6, 2014 at 5:32 PM, Vinson Lee <vlee@freedesktop.org> wrote:
> Fixes Coverity "Unchecked return value" defects.
>
> Signed-off-by: Vinson Lee <vlee@freedesktop.org>
> ---
>  .../spec/ext_unpack_subimage/ext_unpack_subimage.c | 24 +++++++++++-----------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c b/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
> index e65f4be..3935d92 100644
> --- a/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
> +++ b/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
> @@ -156,19 +156,19 @@ piglit_display(void)
>                              0, 0, 1, 1);
>
>         if (extension_supported) {
> -               pass &= piglit_probe_pixel_rgba(piglit_width / 2,
> -                                               piglit_height / 4,
> -                                               blue);
> -               pass &= piglit_probe_pixel_rgba(piglit_width / 2,
> -                                               piglit_height * 3 / 4,
> -                                               cyan);
> +               pass = piglit_probe_pixel_rgba(piglit_width / 2,
> +                                              piglit_height / 4,
> +                                              blue) && pass;
> +               pass = piglit_probe_pixel_rgba(piglit_width / 2,
> +                                              piglit_height * 3 / 4,
> +                                              cyan) && pass;

This is a fine change on its own, but can someone confirm my thinking
that this Coverity defect is just stupid?

How is

pass = pass & func();

not checking the function return?
On Wed, Aug 6, 2014 at 7:49 PM, Matt Turner <mattst88@gmail.com> wrote:
> On Wed, Aug 6, 2014 at 5:32 PM, Vinson Lee <vlee@freedesktop.org> wrote:
>> Fixes Coverity "Unchecked return value" defects.
>>
>> Signed-off-by: Vinson Lee <vlee@freedesktop.org>
>> ---
>>  .../spec/ext_unpack_subimage/ext_unpack_subimage.c | 24 +++++++++++-----------
>>  1 file changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c b/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
>> index e65f4be..3935d92 100644
>> --- a/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
>> +++ b/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
>> @@ -156,19 +156,19 @@ piglit_display(void)
>>                              0, 0, 1, 1);
>>
>>         if (extension_supported) {
>> -               pass &= piglit_probe_pixel_rgba(piglit_width / 2,
>> -                                               piglit_height / 4,
>> -                                               blue);
>> -               pass &= piglit_probe_pixel_rgba(piglit_width / 2,
>> -                                               piglit_height * 3 / 4,
>> -                                               cyan);
>> +               pass = piglit_probe_pixel_rgba(piglit_width / 2,
>> +                                              piglit_height / 4,
>> +                                              blue) && pass;
>> +               pass = piglit_probe_pixel_rgba(piglit_width / 2,
>> +                                              piglit_height * 3 / 4,
>> +                                              cyan) && pass;
>
> This is a fine change on its own, but can someone confirm my thinking
> that this Coverity defect is just stupid?
>
> How is
>
> pass = pass & func();
>
> not checking the function return?

If pass is false, then func() is not necessarily evaluated.
On Thu, Aug 7, 2014 at 10:39 PM, Vinson Lee <vlee@freedesktop.org> wrote:
> On Wed, Aug 6, 2014 at 7:49 PM, Matt Turner <mattst88@gmail.com> wrote:
>> On Wed, Aug 6, 2014 at 5:32 PM, Vinson Lee <vlee@freedesktop.org> wrote:
>>> Fixes Coverity "Unchecked return value" defects.
>>>
>>> Signed-off-by: Vinson Lee <vlee@freedesktop.org>
>>> ---
>>>  .../spec/ext_unpack_subimage/ext_unpack_subimage.c | 24 +++++++++++-----------
>>>  1 file changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c b/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
>>> index e65f4be..3935d92 100644
>>> --- a/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
>>> +++ b/tests/spec/ext_unpack_subimage/ext_unpack_subimage.c
>>> @@ -156,19 +156,19 @@ piglit_display(void)
>>>                              0, 0, 1, 1);
>>>
>>>         if (extension_supported) {
>>> -               pass &= piglit_probe_pixel_rgba(piglit_width / 2,
>>> -                                               piglit_height / 4,
>>> -                                               blue);
>>> -               pass &= piglit_probe_pixel_rgba(piglit_width / 2,
>>> -                                               piglit_height * 3 / 4,
>>> -                                               cyan);
>>> +               pass = piglit_probe_pixel_rgba(piglit_width / 2,
>>> +                                              piglit_height / 4,
>>> +                                              blue) && pass;
>>> +               pass = piglit_probe_pixel_rgba(piglit_width / 2,
>>> +                                              piglit_height * 3 / 4,
>>> +                                              cyan) && pass;
>>
>> This is a fine change on its own, but can someone confirm my thinking
>> that this Coverity defect is just stupid?
>>
>> How is
>>
>> pass = pass & func();
>>
>> not checking the function return?
>
> If pass is false, then func() is not necessarily evaluated.

No, bit-wise operators don't short-circuit.