From: Emil Velikov <emil.veli...@collabora.com> Currently depending on the code path hit, the helper will set some of the output values and not others.
It could also leak memory ;-) At the same time the caller was: - working around the broken behaviour - by initialising the variables - outright ignoring if the helper fails Fix all that by consistently setting the output variables and returning a protocol error if we fail to get the data required. Bonus points: current code does not attribute if we cannot get the modifiers info for certain format. A smaller format array is available, yet the original length is stored. Fixes: cef12efc15c ("glamor: Implement GetSupportedModifiers") Cc: Louis-Francis Ratté-Boulianne <l...@collabora.com> Cc: Daniel Stone <dani...@collabora.com> Signed-off-by: Emil Velikov <emil.veli...@collabora.com> --- We could still return "success" to the user - I've opted for this solution since we already bail on dixLookupWindow failure. --- dri3/dri3_request.c | 18 ++++++++++-------- dri3/dri3_screen.c | 38 +++++++++++++++++++++----------------- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/dri3/dri3_request.c b/dri3/dri3_request.c index edcd0e782..9b7d81ea3 100644 --- a/dri3/dri3_request.c +++ b/dri3/dri3_request.c @@ -335,10 +335,10 @@ proc_dri3_get_supported_modifiers(ClientPtr client) }; WindowPtr window; ScreenPtr pScreen; - CARD64 *window_modifiers = NULL; - CARD64 *screen_modifiers = NULL; - CARD32 nwindowmodifiers = 0; - CARD32 nscreenmodifiers = 0; + CARD64 *window_modifiers; + CARD64 *screen_modifiers; + CARD32 nwindowmodifiers; + CARD32 nscreenmodifiers; int status; int i; @@ -349,10 +349,12 @@ proc_dri3_get_supported_modifiers(ClientPtr client) return status; pScreen = window->drawable.pScreen; - dri3_get_supported_modifiers(pScreen, &window->drawable, - stuff->depth, stuff->bpp, - &nwindowmodifiers, &window_modifiers, - &nscreenmodifiers, &screen_modifiers); + status = dri3_get_supported_modifiers(pScreen, &window->drawable, + stuff->depth, stuff->bpp, + &nwindowmodifiers, &window_modifiers, + &nscreenmodifiers, &screen_modifiers); + if (status != Success) + return status; rep.numWindowModifiers = nwindowmodifiers; rep.numScreenModifiers = nscreenmodifiers; diff --git a/dri3/dri3_screen.c b/dri3/dri3_screen.c index 2b0e139fa..de97d55bc 100644 --- a/dri3/dri3_screen.c +++ b/dri3/dri3_screen.c @@ -178,7 +178,9 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable, int ret; CARD32 num_drawable_mods; CARD64 *drawable_mods; + CARD32 num_window_mods = 0; CARD64 *window_mods = NULL; + CARD32 num_screen_mods = 0; CARD64 *screen_mods = NULL; CARD32 format; dri3_dmabuf_format_ptr screen_format = NULL; @@ -202,11 +204,8 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable, if (screen_format == NULL) return BadMatch; - if (screen_format->num_modifiers == 0) { - *num_screen_modifiers = 0; - *num_window_modifiers = 0; - return Success; - } + if (screen_format->num_modifiers == 0) + goto done; if (!info->get_drawable_modifiers || !info->get_drawable_modifiers(drawable, format, @@ -221,17 +220,13 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable, */ screen_mods = malloc(screen_format->num_modifiers * sizeof(CARD64)); if (!screen_mods) - return BadAlloc; + goto bad_alloc; if (num_drawable_mods > 0) { window_mods = malloc(screen_format->num_modifiers * sizeof(CARD64)); - if (!window_mods) { - free(screen_mods); - return BadAlloc; - } + if (!window_mods) + goto bad_alloc; } - *num_screen_modifiers = 0; - *num_window_modifiers = 0; for (i = 0; i < screen_format->num_modifiers; i++) { CARD64 modifier = screen_format->modifiers[i]; Bool window_mod = FALSE; @@ -245,18 +240,27 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable, if (window_mod) { window_mods[*num_window_modifiers] = modifier; - *num_window_modifiers += 1; + num_window_mods++; } else { screen_mods[*num_screen_modifiers] = modifier; - *num_screen_modifiers += 1; + num_screen_mods++; } } - assert(*num_window_modifiers + *num_screen_modifiers == screen_format->num_modifiers); + assert(num_window_mods + num_screen_mods == screen_format->num_modifiers); - *window_modifiers = window_mods; - *screen_modifiers = screen_mods; +done: free(drawable_mods); + *num_window_modifiers = num_window_mods; + *window_modifiers = window_mods; + *num_screen_modifiers = num_screen_mods; + *screen_modifiers = screen_mods; return Success; + +bad_alloc: + free(drawable_mods); + free(screen_mods); + free(window_mods); + return BadAlloc; } -- 2.16.0 _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel