[Mesa-dev] [PATCH] mesa/st: Avoid a NULL-ptr dereference on possible missing callback

[Ian Romanick]

On 03/28/2016 04:06 AM, eocallaghan at alterapraxis.com wrote:
> Determinism is always better regardless of how you get there. A
> null pointer deference `on purpose` is a really poor idea in C.
> I am somewhat surprised you are asking if its really better to
> rely on undefined behavior vs. an assert.

I think the more important question is how this could get hit.  Asserts
do not exist in release builds.  If the only way the assert could get
hit is by a developer enabling an extension that they didn't actually
implement, it's hard to get very excited about it.

> I thought about using a if branch and just returning but I think
> it is better to crash deterministically with a reason rather than
> perhaps fail silently. Although I do see in other places a if branch
> and a return was used in similar situations so I would be willing to
> do the same if I must.

That's only done in cases where it is valid for the driver to not
provide a callback.

> On 2016-03-28 16:08, Ilia Mirkin wrote:
>> When would that happen? When a user force-enables
>> ARB_query_buffer_object for a driver that's not ready for it? Is
>> hitting a deterministic assert in that case any better than hitting a
>> null deref?
>>
>> On Sun, Mar 27, 2016 at 11:52 PM, Edward O'Callaghan
>> <eocallaghan at alterapraxis.com> wrote:
>>> Just because we miss a gallium driver callback don't dereference
>>> invalid memory.
>>>
>>> Signed-off-by: Edward O'Callaghan <eocallaghan at alterapraxis.com>
>>> ---
>>>  src/mesa/state_tracker/st_cb_queryobj.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/src/mesa/state_tracker/st_cb_queryobj.c
>>> b/src/mesa/state_tracker/st_cb_queryobj.c
>>> index cdb9efc..e9abc38 100644
>>> --- a/src/mesa/state_tracker/st_cb_queryobj.c
>>> +++ b/src/mesa/state_tracker/st_cb_queryobj.c
>>> @@ -402,6 +402,7 @@ st_StoreQueryResult(struct gl_context *ctx,
>>> struct gl_query_object *q,
>>>        index = 0;
>>>     }
>>>
>>> +   assert(pipe->get_query_result_resource);
>>>     pipe->get_query_result_resource(pipe, stq->pq, wait, result_type,
>>> index,
>>>                                     stObj->buffer, offset);
>>>  }
>>> -- 
>>> 2.5.5
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>