[Mesa-dev] [PATCH 1/3] i965: Add a safety check for brw_set_saturate().

[Eric Anholt]

Eric Anholt <eric at anholt.net> writes:

> Kenneth Graunke <kenneth at whitecape.org> writes:
>
>> On 08/08/2012 03:08 PM, Eric Anholt wrote:
>>> There is an easy chance for bugs in brw_wm_emit.c, where you would pass 1 << 5
>>> instead of 1, which would get truncated to 0.
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_eu.c |    1 +
>>>  1 file changed, 1 insertion(+)
>>> 
>>> diff --git a/src/mesa/drivers/dri/i965/brw_eu.c b/src/mesa/drivers/dri/i965/brw_eu.c
>>> index 2c432a9..7697d08 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_eu.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_eu.c
>>> @@ -143,6 +143,7 @@ void brw_set_mask_control( struct brw_compile *p, GLuint value )
>>>  
>>>  void brw_set_saturate( struct brw_compile *p, GLuint value )
>>>  {
>>> +   assert(value == 0 || value == 1);
>>>     p->current->header.saturate = value;
>>>  }
>>
>> Eesh.  It sounds like what you really want is:
>>
>> #include <stdbool.h>
>>
>> void
>> brw_set_saturate(struct brw_compile *p, bool value)
>> {
>>    p->current->header.saturate = value;
>> }
>>
>> Then zero/non-zero would work as expected.
>
> Oh, cool.  I hadn't realized that stdbool actually did nonzero -> true
> conversion exactly the way we want, which is why I hadn't just done this.

Good point, I hadn't realized the int -> bool conversion worked the way
you would hope.  I've switched the first commit to do that, but I left
the to-be-merged-to-8.0 one as-is with != 0 comparisons, since it's
stylistically consistent with the rest of the file.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120808/0945e39a/attachment.pgp>