Created attachment 326020 [details] [review]
st: Do not try to unref NULL CoglObjects

Created attachment 326021 [details] [review]
Do not skip CoglError parameters

While CoglError is a define to GError, it doesn't follow the convention
of ignoring errors when NULL is passed, but rather treats the error as
fatal :-(
That's clearly unwanted for a compositor, so make sure to always pass
an error parameter where a runtime error is possible

Review of attachment 326020 [details] [review]:

::: src/st/st-private.c
@@ +397,3 @@
   cogl_pipeline_set_layer_texture (pipeline, 0, texture);
+
+  if (texture)

I don't think cogl_pipeline_copy can fail unless we're OOM... In fact there are other places that don't check for NULL being returned from cogl_pipeline_new() calls

::: src/st/st-texture-cache.c
@@ +643,3 @@
       clutter_texture_set_cogl_texture (bind->texture, texdata);
+
+      if (texdata)

might as well skip the clutter call since that will just print a warning

Review of attachment 326021 [details] [review]:

::: src/st/st-theme-node-drawing.c
@@ +184,2 @@
   g_free (data);
   g_assert (texture != COGL_INVALID_HANDLE);

:-(

(In reply to Rui Matos from comment #3)
>    cogl_pipeline_set_layer_texture (pipeline, 0, texture);
> +
> +  if (texture)
> 
> I don't think cogl_pipeline_copy can fail unless we're OOM... In fact there
> are other places that don't check for NULL being returned from
> cogl_pipeline_new() calls

The check is for the return value of cogl_texture_2d_new_from_data(), not cogl_pipeline_new/copy().


> ::: src/st/st-texture-cache.c
> +      if (texdata)
> 
> might as well skip the clutter call since that will just print a warning

OK.

Created attachment 326113 [details] [review]
st: Do not try to unref NULL CoglObjects

Review of attachment 326113 [details] [review]:

lgtm

::: src/st/st-private.c
@@ +397,3 @@
   cogl_pipeline_set_layer_texture (pipeline, 0, texture);
+
+  if (texture)

right, I misread this before

Attachment 326021 [details] pushed as 63f6ff9 - Do not skip CoglError parameters
Attachment 326113 [details] pushed as 59c2ace - st: Do not try to unref NULL CoglObjects