[Mesa-dev] [PATCH 4/4] mesa/main: drop needless condition check in _mesa_GetPerfCounterInfoINTEL

[Lionel Landwerlin]

On 31/05/17 02:04, Ian Romanick wrote:
> I've seen this warning a million times, and I always talk myself out of
> fixing it.  If the type of counterIndex ever changes to int (or another
> signed type), this will break.  It may not be obvious at the time of the
> break that it happened, and it will likely be annoying to debug.
>
> I'm not NAKing it, but it makes me nervous.  What do other folks think?

Spec says :

"

Performance counter id 0
     is reserved as an invalid counter.

"

Maybe we should change counterid_to_index() to return a signed integer?
What are the chances to have more than 1^31 counters?

>
> On 05/30/2017 01:47 PM, Rob Herring wrote:
>> Fixes the following warning:
>>
>> external/mesa3d/src/mesa/main/performance_query.c:352:21: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
>>     if (counterIndex < 0 || counterIndex >= queryNumCounters) {
>>         ~~~~~~~~~~~~ ^ ~
>>
>> counterIndex will be ~0 if invalid and the 2nd condition is sufficient to
>> catch that case.
>>
>> Signed-off-by: Rob Herring <robh at kernel.org>
>> ---
>>   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 56f6a7da8b9f..b9e7cf9961b5 100644
>> --- a/src/mesa/main/performance_query.c
>> +++ b/src/mesa/main/performance_query.c
>> @@ -349,7 +349,7 @@ _mesa_GetPerfCounterInfoINTEL(GLuint queryId, GLuint counterId,
>>   
>>      counterIndex = counterid_to_index(counterId);
>>   
>> -   if (counterIndex < 0 || counterIndex >= queryNumCounters) {
>> +   if (counterIndex >= queryNumCounters) {
>>         _mesa_error(ctx, GL_INVALID_VALUE,
>>                     "glGetPerfCounterInfoINTEL(invalid counterId)");
>>         return;
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170531/7f0d4741/attachment.html>