[Mesa-dev] [PATCH] st/mesa: use signed temporary variable to store _ColorDrawBufferIndexes

[Emil Velikov]

On 15/01/14 11:49, Emil Velikov wrote:
> The temporary variable used to store _ColorDrawBufferIndexes must be
> signed (GLint), otherwise the following conditional will be incorrectly
> evaluated. Leading to crashes in the driver/mesa or accessing/writing
> to arbitrary memory location. The bug dates back to 2009.
> 
> Cc: 10.0 9.2 9.1 <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> ---
> 
> Rather old bug, spotted after Marek's recent patches covering the
> area. Curious if there is any particular reason why we do not
> enable more compiler warning messages.
> 
> How do people feel on the subject of enabling more (all even) compiler
> warnings on gcc compatible compilers ?
> 
Actually Wall is already set, so I'll give -Wextra -Wconversion a try.
Most places in mesa already have explicit casts, which should not be
affected by the additional compiler flags.

~
Emil

> Just for laughs I'll set -Wall locally to see how many warning
> messages gcc will produce. At least some of those would be usefull.
> 
> Cheers,
> Emil
> ---
>  src/mesa/state_tracker/st_cb_clear.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/state_tracker/st_cb_clear.c b/src/mesa/state_tracker/st_cb_clear.c
> index 87dccee..79124b3 100644
> --- a/src/mesa/state_tracker/st_cb_clear.c
> +++ b/src/mesa/state_tracker/st_cb_clear.c
> @@ -444,7 +444,7 @@ st_Clear(struct gl_context *ctx, GLbitfield mask)
>  
>     if (mask & BUFFER_BITS_COLOR) {
>        for (i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++) {
> -         GLuint b = ctx->DrawBuffer->_ColorDrawBufferIndexes[i];
> +         GLint b = ctx->DrawBuffer->_ColorDrawBufferIndexes[i];
>  
>           if (b >= 0 && mask & (1 << b)) {
>              struct gl_renderbuffer *rb
>