On 09/19/2013 03:41 PM, Kenneth Graunke wrote:
> If the last counter returned is a 32-bit value, reading a uint64_t might
> go past the end of the buffer.  So, delay reading the 64-bit value until
> we know the type is 64-bit.
> 
> Signed-off-by: Kenneth Graunke <[email protected]>

Reviewed-by: Ian Romanick <[email protected]>

Some semi-random thoughts below...

> ---
>  tests/spec/amd_performance_monitor/measure.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/spec/amd_performance_monitor/measure.c 
> b/tests/spec/amd_performance_monitor/measure.c
> index bc39c84..61c98b1 100644
> --- a/tests/spec/amd_performance_monitor/measure.c
> +++ b/tests/spec/amd_performance_monitor/measure.c
> @@ -194,7 +194,6 @@ test_basic_measurement(unsigned group)
>               /* Counter values */
>               uint32_t u32 = p[2];
>               float f = ((float *) p)[2];
> -             uint64_t u64 = ((uint64_t *) p)[1];
>  
>               /* Query results */
>               GLenum counter_type = GL_NONE;
> @@ -226,6 +225,7 @@ test_basic_measurement(unsigned group)
>                       break;
>               }
>               case GL_UNSIGNED_INT64_AMD: {
> +                     uint64_t u64 = ((uint64_t *) p)[1];

This looks so weird to me, given that the others use [2].  The
alternative is also a face only a mother could love:

        uint64_t u64 = *(uint64_t *)(p + 2);

Of course, then the other usages could follow the same pattern:

                /* Counter values */
                uint32_t u32 = *(uint32_t *)(p + 2);
                float f = *(float *)(p + 2);

What do you think?

>                       verify(u64 >= range[0]);
>                       verify(u64 <= range[1]);
>                       break;
> 

_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to