[OE-core] [PATCH] cogl: fix compile error with -Werror=maybe-uninitialized
Changqing Li
changqing.li at windriver.com
Thu Apr 4 02:06:44 UTC 2019
On 4/3/19 5:58 PM, Adrian Bunk wrote:
> On Wed, Apr 03, 2019 at 10:29:08AM +0800, Changqing Li wrote:
>> On 4/2/19 7:31 PM, Adrian Bunk wrote:
>>> On Tue, Apr 02, 2019 at 05:08:33PM +0800, changqing.li at windriver.com wrote:
>>>> From: Changqing Li <changqing.li at windriver.com>
>>>>
>>>> fix below compile error with -Werror=maybe-uninitialized
>>>>
>>>> | ../../cogl-1.22.2/cogl/driver/gl/gles/cogl-driver-gles.c:217:17: error: 'gltype' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>>> | *out_gltype = gltype;
>>>> | ~~~~~~~~~~~~^~~~~~~~
>>>> | ../../cogl-1.22.2/cogl/driver/gl/gles/cogl-driver-gles.c:213:22: error: 'glintformat' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>>> | *out_glintformat = glintformat;
>>>> | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
>>>> ...
>>> Looking at the code, is this a failure that only happens with DEBUG_FLAGS?
>> Yes, only happen with DEBUG_FLAGS
>>>> +--- a/cogl/driver/gl/gles/cogl-driver-gles.c
>>>> ++++ b/cogl/driver/gl/gles/cogl-driver-gles.c
>>>> +@@ -74,9 +74,9 @@ _cogl_driver_pixel_format_to_gl (CoglContext *context,
>>>> + GLenum *out_gltype)
>>>> + {
>>>> + CoglPixelFormat required_format;
>>>> +- GLenum glintformat;
>>>> ++ GLenum glintformat = 0;
>>>> + GLenum glformat = 0;
>>>> +- GLenum gltype;
>>>> ++ GLenum gltype = 0;
>>>> ...
>>> Assigning random values to variables is a bug,
>>> they do not even seem to be valid values for these variables.
>> According to code logic, glformat and glintformat will assigned
>> simultaneously and usually with same value,
> But glformat has an assert later to ensure the 0 doesn't leak anywhere.
>
>> and 0 mean invalid pixel format.
> For pixel format 0 means COGL_PIXEL_FORMAT_ANY.
>
> For glintformat I don't know what would happen if 0 is used.
>
>> [snip]
>>
>> case COGL_PIXEL_FORMAT_DEPTH_24_STENCIL_8:
>> glintformat = GL_DEPTH_STENCIL;
>> glformat = GL_DEPTH_STENCIL;
>> gltype = GL_UNSIGNED_INT_24_8;
>> break;
>>
>> and for gtype, there also should be no problem according to the code logic.
> According to the current code.
>
> If some future change breaks the code, it would pass invalid data at
> runtime instead of giving a compile warning.
>
>> and the fix is just for eliminate the error when DEBUG_FLAGS is enabled.
> Looking at the code, does passing --enable-maintainer-flags=no
> to configure work around this gcc bug
Hi, Adrian
Thanks for your detailed comments. And the future change wrong is
reasonable, but according to
current code, seems it can assert from g_assert_not_reached or g_assert
when glformat/glintformat is not assigned new value.
[snip]
case xxx
glformat=xxx
glintformat=xxx
case COGL_PIXEL_FORMAT_ANY:
case COGL_PIXEL_FORMAT_YUV:
g_assert_not_reached ();
break;
}
/* All of the pixel formats are handled above so if this hits then
we've been given an invalid pixel format */
g_assert (glformat != 0);
And also upstream have use same way to fix under
cogl/driver/gl/gl/cogl-driver-gl.c
https://gitlab.gnome.org/GNOME/cogl/commit/ca5226513eb64bceb38ca01994799c4e7cd9f5fa
so I think we can fix same way under this
cogl/driver/gl/gles/cogl-driver-gles.c
>
> cu
> Adrian
>
--
BRs
Sandy(Li Changqing)
More information about the Openembedded-core
mailing list