Created attachment 299275 [details] [review]
popover: Fix crash when popover is NULL

When the popover was NULL, we'd access invalid memory before the
g_return_if_fail guards.

Review of attachment 299275 [details] [review]:

Oops. Looks good to me

Review of attachment 299275 [details] [review]:

We generally don't accept NULL as a valid argument where an object is expected, unless it is explicitly annotated.

Which is why these accesses to priv pointers are safe (we do them all over the place)

And our policy for hitting a return_if_fail is: it might as well have crashed.

First time I hear that. What's wrong with failing gracefully?

(In reply to Bastien Nocera from comment #6)
> First time I hear that. What's wrong with failing gracefully?

Considering that GLib (and basically every other library in the G* stack using GLib for precondition checks) can be compiled with all the g_return_* macros disabled, you will get a crash anyway later on — and not even a critical warning.

In general, and in accordance to what Matthias said in comment #4, the private data access should be done through the get_instance_private() accessor, instead of using a priv pointer, as that is NULL-instance-pointer safe.

(In reply to Bastien Nocera from comment #6)
> First time I hear that. What's wrong with failing gracefully?

Nothing wrong with it. I just don't want to establish prior art that would render

func (Foo *foo,...)
{
  FooPrivate *priv = foo->priv;
}

suspect. I use that idiom a lot, and don't want to start getting patches to eliminate it all over gtk...

*** Bug 755499 has been marked as a duplicate of this bug. ***

Timm fixed these in commit 0d17421ffdc3b6d3d47eaa38791f84e6111fe3f0