[Mesa-dev] [PATCH v2] egl/dri2: Fix GCC maybe-uninitialized warning.

[Ilia Mirkin]

On Wed, Mar 25, 2015 at 8:31 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Wed, Mar 25, 2015 at 4:15 PM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
>> On Wed, 2015-03-25 at 18:55 -0400, Ilia Mirkin wrote:
>>> On Wed, Mar 25, 2015 at 6:51 PM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
>>> > On Fri, 2015-03-06 at 23:54 -0800, Vinson Lee wrote:
>>> >> egl_dri2.c: In function ‘dri2_bind_tex_image’:
>>> >> egl_dri2.c:1240:4: warning: ‘format’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>> >>     (*dri2_dpy->tex_buffer->setTexBuffer2)(dri2_ctx->dri_context,
>>> >>     ^
>>> >>
>>> >> Suggested-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> >> Signed-off-by: Vinson Lee <vlee at freedesktop.org>
>>> >> ---
>>> >>  src/egl/drivers/dri2/egl_dri2.c | 6 ++++--
>>> >>  1 file changed, 4 insertions(+), 2 deletions(-)
>>> >>
>>> >> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
>>> >> index d503196..c5c475d 100644
>>> >> --- a/src/egl/drivers/dri2/egl_dri2.c
>>> >> +++ b/src/egl/drivers/dri2/egl_dri2.c
>>> >> @@ -1226,7 +1226,8 @@ dri2_bind_tex_image(_EGLDriver *drv,
>>> >>        format = __DRI_TEXTURE_FORMAT_RGBA;
>>> >>        break;
>>> >>     default:
>>> >> -      assert(0);
>>> >> +      _eglError(EGL_BAD_SURFACE, "unrecognized format");
>>> >> +      return EGL_FALSE;
>>> >
>>> > does using:
>>> > unreachable("unrecognized format");
>>> > instead of
>>> > assert(0);
>>> > fix the warning?
>>>
>>> unreachable is for *truly* unreachable code... it sounded like this
>>> was reachable with bad input.
>>
>> maybe I misunderstood the situation.
>> since there is assert(0) I assumed it can never happen.
>> combination of assert(0) and return is very confusing:
>> either the code is reachable and should have a correct error path (in
>> which case there should not be assert(0)),
>> or the code is not reachable in which case unreachable does just fine
>> and you should not have the error path.
>>
>> it looks to me that using assert and return just makes sure that the
>> error path is never run on debug build.
>>
>> anyway, it was just a suggestion. I won't argue one way or another,
>> since I don't work with/understand those parts of the code.
>
> I agree. If the code had an assert(0) it's pretty clearly a case for
> unreachable.

I dunno, I've seen assert's thrown in all over the place where the
assumption was that they'd only trigger on debug builds. Not sure if
this is one of those cases, but I have a hard time convincing myself
that there's no way an unexpected value can get in there. The downside
of unreachable() is that it ends up as an infinite loop or other sorts
of funny control flow, which can be quite difficult to debug (in a
production build).