[v2] arb_internalformat_query2: Set expected_value in default case.

Submitted by Vinson Lee on March 8, 2016, 6:37 a.m.

Details

Message ID 1457419026-11033-1-git-send-email-vlee@freedesktop.org
State New
Headers show
Series "arb_internalformat_query2: Set expected_value in default case." ( rev: 2 ) in Piglit

Not browsing as part of any series.

Commit Message

Vinson Lee March 8, 2016, 6:37 a.m.
Fixes uninitialized scalar defect reported by Coverity.

Also fix incorrect assertion.

Suggested-by: Brian Paul <brianp@vmware.com>
Signed-off-by: Vinson Lee <vlee@freedesktop.org>
---
 tests/spec/arb_internalformat_query2/format-components.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/tests/spec/arb_internalformat_query2/format-components.c b/tests/spec/arb_internalformat_query2/format-components.c
index 10d7f52..3b8cf35 100644
--- a/tests/spec/arb_internalformat_query2/format-components.c
+++ b/tests/spec/arb_internalformat_query2/format-components.c
@@ -217,7 +217,8 @@  try(const GLenum *targets, unsigned num_targets,
                                         expected_value = is_depth_format(internalformats[j]);
                                         break;
                                 default:
-                                        assert("incorrect pname for test");
+                                        assert(!"incorrect pname for test");
+                                        expected_value = 0;
                                         break;
                                 }
 

Comments

On 08/03/16 07:37, Vinson Lee wrote:
> Fixes uninitialized scalar defect reported by Coverity.
>
> Also fix incorrect assertion.
>
> Suggested-by: Brian Paul <brianp@vmware.com>
> Signed-off-by: Vinson Lee <vlee@freedesktop.org>
> ---
>  tests/spec/arb_internalformat_query2/format-components.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/spec/arb_internalformat_query2/format-components.c b/tests/spec/arb_internalformat_query2/format-components.c
> index 10d7f52..3b8cf35 100644
> --- a/tests/spec/arb_internalformat_query2/format-components.c
> +++ b/tests/spec/arb_internalformat_query2/format-components.c
> @@ -217,7 +217,8 @@ try(const GLenum *targets, unsigned num_targets,
>                                          expected_value = is_depth_format(internalformats[j]);
>                                          break;
>                                  default:
> -                                        assert("incorrect pname for test");
> +                                        assert(!"incorrect pname for test");
> +                                        expected_value = 0;
>                                          break;
>                                  }
>  

I would prefer a value that is clearly wrong, like -1 and initialize the
variable to -1 instead of adding a fake assignment on default.
Additionally, both things is what it is done on image-texture test, so
it is probably more consistent this way.

With those small changes:

Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>

Thanks.
On 08/03/16 09:03, Alejandro Piñeiro wrote:
>
> On 08/03/16 07:37, Vinson Lee wrote:
>> Fixes uninitialized scalar defect reported by Coverity.
>>
>> Also fix incorrect assertion.
>>
>> Suggested-by: Brian Paul <brianp@vmware.com>
>> Signed-off-by: Vinson Lee <vlee@freedesktop.org>
>> ---
>>  tests/spec/arb_internalformat_query2/format-components.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/spec/arb_internalformat_query2/format-components.c b/tests/spec/arb_internalformat_query2/format-components.c
>> index 10d7f52..3b8cf35 100644
>> --- a/tests/spec/arb_internalformat_query2/format-components.c
>> +++ b/tests/spec/arb_internalformat_query2/format-components.c
>> @@ -217,7 +217,8 @@ try(const GLenum *targets, unsigned num_targets,
>>                                          expected_value = is_depth_format(internalformats[j]);
>>                                          break;
>>                                  default:
>> -                                        assert("incorrect pname for test");
>> +                                        assert(!"incorrect pname for test");
>> +                                        expected_value = 0;
>>                                          break;
>>                                  }
>>  
> I would prefer a value that is clearly wrong, like -1 and initialize the
> variable to -1 instead of adding a fake assignment on default.
> Additionally, both things is what it is done on image-texture test, so
> it is probably more consistent this way.

BTW, I have realized that image-texture has the same mistake on the
assert. It would be awesome if you fix that issue too. You can do it on
the same patch or a different one, as you prefer.

>
> With those small changes:
>
> Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
>
> Thanks.
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit