On Fri, 14 Feb 2014, Henri Verbeet wrote:
On 13 February 2014 13:05, Martin Storsjo <martin(a)martin.st> wrote:
static HRESULT arbfp_blit_alloc(struct wined3d_device *device) { + struct arbfp_blit_priv *priv; device->blit_priv = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(struct arbfp_blit_priv)); if(!device->blit_priv) { ERR("Out of memory\n"); return E_OUTOFMEMORY; } + priv = device->blit_priv; + + if (wine_rb_init(&priv->shaders, &wined3d_arbfp_blit_rb_functions) == -1) + { + ERR("Failed to initialize rbtree.\n"); + HeapFree(GetProcessHeap(), 0, device->blit_priv); + device->blit_priv = NULL; + return E_OUTOFMEMORY; + } + return WINED3D_OK; }
The following is probably slightly nicer:
static HRESULT arbfp_blit_alloc(struct wined3d_device *device) { - device->blit_priv = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(struct arbfp_blit_priv)); - if(!device->blit_priv) { - ERR("Out of memory\n"); + struct arbfp_blit_priv *priv; + + if (!(priv = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*priv)))) return E_OUTOFMEMORY; + + if (wine_rb_init(&priv->shaders, &wined3d_arbfp_blit_rb_functions) == -1) + { + ERR("Failed to initialize rbtree.\n"); + HeapFree(GetProcessHeap(), 0, priv); + return E_FAIL; } + + device->blit_priv = priv; + return WINED3D_OK; }
That's nicer indeed, updated the patch accordingly locally.
+ if (desc) + HeapFree(GetProcessHeap(), 0, desc); There's no point in checking for NULL before HeapFree().
MSDN says "If this pointer is NULL, the behavior is undefined" about the lpMem parameter to HeapFree.
Perhaps more importantly, I think it makes more sense to insert the rbtree entries in arbfp_blit_set(), instead of duplicating the code between gen_p8_shader() and gen_yuv_shader().
Good point, I've updated my patch accordingly. I'll resend all three patches in a little while unless you or Stefan have got more comments at the moment. // Martin