[Mesa-dev] [PATCH] Mesa: Fix performance query id check

[Robert Bragg]

On Mon, Feb 27, 2017 at 2:47 PM, Juha-Pekka Heikkila
<juhapekka.heikkila at gmail.com> wrote:
> In queryid_valid() fix handling of zero index.
>
> CC: Robert Bragg <robert at sixbynine.org>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
>  src/mesa/main/performance_query.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/performance_query.c b/src/mesa/main/performance_query.c
> index aa10351..142d834 100644
> --- a/src/mesa/main/performance_query.c
> +++ b/src/mesa/main/performance_query.c
> @@ -91,7 +91,7 @@ static inline bool
>  queryid_valid(const struct gl_context *ctx, unsigned numQueries, GLuint queryid)
>  {
>     GLuint index = queryid_to_index(queryid);
> -   return index >= 0 && index < numQueries;
> +   return queryid != 0 && index < numQueries;
>  }
>
>  static inline GLuint
> --
> 2.7.4
>

Oh, I thought I'd sent out an updated patch, but apparently I made it
but got distracted and didn't actually send it :-/

Main differences with mine was that I added a quote from the spec
about ID zero being reserved:

-   GLuint index = queryid_to_index(queryid);
-   return index >= 0 && index < numQueries;
+   /* The GL_INTEL_performance_query spec says:
+    *
+    *  "Performance counter ids values start with 1. Performance counter id 0
+    *  is reserved as an invalid counter."
+    */
+   return queryid != 0 && queryid_to_index(queryid) < numQueries;

And dropped the local variable which you say you'd prefer to keep. I
suppose on the other hand it seems good to me to not pass an index of
zero to queryid_to_index() before validation in case
queryid_to_index() might one day gain its own paranoid assertion that
the queryid is >= 0.

I don't have a strong opinion here though, so your patch looks fine to
land to me:
Reviewed-by: Robert Bragg <robert at sixbynine.org>

Br,
- Robert