Re: [PATCHv4 2/3] wined3d: Use an rbtree for storing shaders for texture format conversion/blitting

[Henri Verbeet]

On 13 February 2014 13:05, Martin Storsjo  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;
 }
> - if (desc)
> -
>    HeapFree(GetProcessHeap(), 0, desc);

There's no point in checking for NULL before 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().