Since seeing that I had indeed sent out a v2 patch and it was reviewed
by Plamena too (thanks) I've just gone a head and pushed that now
(though with an updated commit message instead of copy pasting the
original message).
Thanks,
- Robert
On Mon, Feb 27, 2017 at 3:43 PM, Robert Bragg <rob...@sixbynine.org> wrote:
> On Mon, Feb 27, 2017 at 2:47 PM, Juha-Pekka Heikkila
> <juhapekka.heikk...@gmail.com> wrote:
>> In queryid_valid() fix handling of zero index.
>>
>> CC: Robert Bragg <rob...@sixbynine.org>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikk...@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 <rob...@sixbynine.org>
>
> Br,
> - Robert
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
