[Mesa-dev] [PATCH 10/13] mesa/main: Check context pointer in _mesa_error before using it

[Ian Romanick]

On 05/07/2015 05:17 AM, Pohjolainen, Topi wrote:
> On Tue, May 05, 2015 at 02:25:26PM +0300, Juha-Pekka Heikkila wrote:
>> I guess this should not really be able to segfault but still it
>> seems to be able to during context creation.
>>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>> ---
>>  src/mesa/main/errors.c | 26 ++++++++++++++++----------
>>  1 file changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
>> index 2aa1deb..6631b82 100644
>> --- a/src/mesa/main/errors.c
>> +++ b/src/mesa/main/errors.c
>> @@ -1458,18 +1458,23 @@ _mesa_error( struct gl_context *ctx, GLenum error, const char *fmtString, ... )
>>  
> To me it looks that it would be better to just leave early already here:
> 
>       if (!ctx)
>          return;
> 
> Avoids extra indentation and it doesn't look meaningful to call
> should_output() with null context.

I like that plan.

I don't think you can even get to _mesa_error (or _mesa_warning) without
a context.  Maybe add an assert(ctx != NULL)?

>>     do_output = should_output(ctx, error, fmtString);
>>  
>> -   mtx_lock(&ctx->DebugMutex);
>> -   if (ctx->Debug) {
>> -      do_log = debug_is_message_enabled(ctx->Debug,
>> -                                        MESA_DEBUG_SOURCE_API,
>> -                                        MESA_DEBUG_TYPE_ERROR,
>> -                                        error_msg_id,
>> -                                        MESA_DEBUG_SEVERITY_HIGH);
>> +   if (ctx) {
>> +      mtx_lock(&ctx->DebugMutex);
>> +      if (ctx->Debug) {
>> +         do_log = debug_is_message_enabled(ctx->Debug,
>> +                                           MESA_DEBUG_SOURCE_API,
>> +                                           MESA_DEBUG_TYPE_ERROR,
>> +                                           error_msg_id,
>> +                                           MESA_DEBUG_SEVERITY_HIGH);
>> +      }
>> +      else {
>> +         do_log = GL_FALSE;
>> +      }
>> +      mtx_unlock(&ctx->DebugMutex);
>>     }
>>     else {
>>        do_log = GL_FALSE;
>>     }
>> -   mtx_unlock(&ctx->DebugMutex);
>>  
>>     if (do_output || do_log) {
>>        char s[MAX_DEBUG_MESSAGE_LENGTH], s2[MAX_DEBUG_MESSAGE_LENGTH];
>> @@ -1502,14 +1507,15 @@ _mesa_error( struct gl_context *ctx, GLenum error, const char *fmtString, ... )
>>        }
>>  
>>        /* Log the error via ARB_debug_output if needed.*/
>> -      if (do_log) {
>> +      if (ctx && do_log) {
>>           log_msg(ctx, MESA_DEBUG_SOURCE_API, MESA_DEBUG_TYPE_ERROR,
>>                   error_msg_id, MESA_DEBUG_SEVERITY_HIGH, len, s2);
>>        }
>>     }
>>  
>>     /* Set the GL context error state for glGetError. */
>> -   _mesa_record_error(ctx, error);
>> +   if (ctx)
>> +      _mesa_record_error(ctx, error);
>>  }
>>  
>>  void
>> -- 
>> 1.8.5.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>