[Mesa-dev] [PATCH 2/2] mesa: add typesize validation to resource functions

[Tapani Pälli]

On 08/11/2017 01:02 PM, Timothy Arceri wrote:
> 
> 
> On 11/08/17 17:45, Tapani Pälli wrote:
>> This makes development/changes to program resource code more safe.
>> Patch also makes helper functions static.
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/mesa/main/shader_query.cpp | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/shader_query.cpp 
>> b/src/mesa/main/shader_query.cpp
>> index f2bdcaa..d1fc5e9 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -49,8 +49,9 @@ program_resource_location(struct gl_program_resource 
>> *res,
>>    * Warning! this is not type safe so be *very* careful when using 
>> these.
>>    */
>>   #define DECL_RESOURCE_FUNC(name, type) \
>> -const type * RESOURCE_ ## name (gl_program_resource *res) { \
>> +static const type * RESOURCE_ ## name (gl_program_resource *res) { \
>>      assert(res->Data); \
>> +   assert(res->TypeSize == sizeof(type));\
> 
> I'm not really a fan of adding members and passing them around just for 
> the sake of debug builds.

Fair enough, this was meant to bring some additional safety as one can 
use the functions with wrong type without any errors (there was actual 
bug about this when adding the initial code where wrong type was used 
with some resource and it was very hard to track down, cannot seem to 
find it now though).

> What exactly is the problem you are running into? Can it be solved by 
> using the existing members?
> 
> Could you just do this instead?
> 
> #define DECL_RESOURCE_FUNC(name, type, data_type) \
> const type * RESOURCE_ ## name (gl_program_resource *res) { \
>     assert(res->Type == type); \
>     assert(res->Data); \
>     return (type *) res->Data; \
> }
> 
> DECL_RESOURCE_FUNC(ATC, GL_ATOMIC_COUNTER_BUFFER,
>                     gl_active_atomic_buffer);
> 
> 
> 
>>      return (type *) res->Data; \
>>   }
>>

Sure, this looks fine approach to me, will make it more safe!

// Tapani