Sounds good, is there a protocol to signify a patch isn't active anymore? <div class="gmail_quot<blockquote class=" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 03/09/2016 04:37 PM, Lars Hamre wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I have not been able to force a NULL dereference, this is based off analyzing the code. Yes that is implicitly true, but if at some point the implicit relationship is broken, I would rather not have a NULL dereference. If you do not agree, I am fine deferring to your judgement! </blockquote> I don't think the code in question will be hit if texObj is null. This code is pretty old (perhaps 15 years) but has been used a lot. I think we'd have known by now if there was a null pointer problem. -Brian <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On Wed, Mar 9, 2016 at 6:23 PM, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a> <mailto:<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>>> wrote: On 03/09/2016 10:21 AM, Lars Hamre wrote: > Fixes a possible null dereference. > > NOTE: this is my first time contributing, please let me know if I > should be doing anything differently, thanks! > > Signed-off-by: Lars Hamre <<a href="mailto:chemecse@gmail.com" target="_blank">chemecse@gmail.com</a> <mailto:<a href="mailto:chemecse@gmail.com" target="_blank">chemecse@gmail.com</a>>> > --- > src/mesa/swrast/s_triangle.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/src/mesa/swrast/s_triangle.c b/src/mesa/swrast/s_triangle.c > index 876a74b..9225974 100644 > --- a/src/mesa/swrast/s_triangle.c > +++ b/src/mesa/swrast/s_triangle.c > @@ -781,7 +781,7 @@ fast_persp_span(struct gl_context *ctx, SWspan *span, > } > break; > } > - > + > assert(span->arrayMask & SPAN_RGBA); > _swrast_write_rgba_span(ctx, span); > > @@ -1063,8 +1063,8 @@ _swrast_choose_triangle( struct gl_context *ctx ) > swImg = swrast_texture_image_const(texImg); > > format = texImg ? texImg->TexFormat : MESA_FORMAT_NONE; > - minFilter = texObj2D ? samp->MinFilter : GL_NONE; > - magFilter = texObj2D ? samp->MagFilter : GL_NONE; > + minFilter = (texObj2D && samp) ? samp->MinFilter : GL_NONE; > + magFilter = (texObj2D && samp) ? samp->MagFilter : GL_NONE; NAK this hunk. If texObj2D is not NULL, samp is also not NULL. > envMode = ctx->Texture.Unit[0].EnvMode; > > /* First see if we can use an optimized 2-D texture function */ > @@ -1073,6 +1073,7 @@ _swrast_choose_triangle( struct gl_context *ctx ) > && !ctx->ATIFragmentShader._Enabled > && ctx->Texture._MaxEnabledTexImageUnit == 0 > && ctx->Texture.Unit[0]._Current->Target == GL_TEXTURE_2D > + && samp I think the 'ctx->Texture.Unit[0]._Current->Target == GL_TEXTURE_2D' implicitly ensures that samp cannot be NULL. Have you been able to cause a NULL dereference in this code path or is this just based on speculation? > && samp->WrapS == GL_REPEAT > && samp->WrapT == GL_REPEAT > && texObj2D->_Swizzle == SWIZZLE_NOOP > -- > 2.5.0 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <mailto:<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a>> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote> </div>