Created attachment 263112 [details] [review]
cogl-framebuffer: Fix a potential NULL pointer dereference

Require that [a|b]->clip_state.stacks both be non-NULL before trying to
compare their data fields.

Found by scan-build.

Hrmm, I think it should be safe to assume that the stack of clip stacks is never empty so this should never cause a problem. At least other parts of the code make that assumption. But yes, that bit of code is very odd and would crash if both clip stacks were empty.

I think we don't actually need the stack of stacks any more so it might be less messy to just remove the whole thing. I'll attach a patch to do that.

Thanks for reporting this.

Created attachment 263123 [details] [review]
Remove the framebuffer's stack of clip stacks

There used to be a function called cogl_clip_stack_save in the public
API which was used when temporarily switching to an offscreen buffer
to save the clip state. This is no longer necessary because each
framebuffer has its own clip stack anyway so the function was removed
in master. However the code to maintain the stack of stacks was
retained. This patch removes it in an effort to simplify the code.

On the 1.18 branch this function is deprecated and the documentation
says that it does nothing. However that is incorrect because it does
actually the push clip stack. I think it would be safe to backport
this patch to the 1.18 branch and actually make it do nothing like it
is documented to do.

Created attachment 263130 [details] [review]
Patch for cogl 1.18

Here is the same patch ported to the cogl-1.18 branch. It has the following additional comment:

“This patch has some extra changes while backporting to the 1.18
 branch. Here the cogl-clip-state file still contained some deprecated
 functions. Instead of deleting the file completely it has been moved
 to the deprecated folder. The declarations for these functions have
 been moved from cogl1-context.h to a new deprecated/cogl-clip-state.h
 header.”

Cool, it's always nice to delete redundant code.

Both of these patches look good to land to me:

Reviewed-by: Robert Bragg <robert@linux.intel.com>

Ok, thanks. I've pushed them both to their respective branches.