[PATCH weston 5/6] libweston-desktop/xwayland: XWAYLAND surfaces are never 'added'

[Pekka Paalanen]

On Thu, 24 Nov 2016 16:02:14 +0100
Quentin Glidic <sardemff7+wayland at sardemff7.net> wrote:

> On 24/11/2016 12:40, Pekka Paalanen wrote:
> > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> >
> > Add documentation (asserts) that show that windows of types XWAYLAND are
> > never registered with the shell.
> >
> > Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>  
> 
> Not a big fan of asserts in internal stuff, but if you feel like it’s 
> needed:
> Reviewed-by: Quentin Glidic <sardemff7+git at sardemff7.net>

Heh, my view is quite the opposite.

Internal stuff is where the asserts actually belong in. If internal
stuff is misused or buggy, we want to know about it. Internal API is
just as important as external API, even if it does not follow the same
stability rules. In fact, because internal API does not follow any
stability rules, it is much more important to sprinkle lots of asserts
to catch misuse, since the patterns of proper use change much more
likely.

OTOH, external APIs often want to return errors instead of crashing the
whole program, so asserts are slightly less appropriate there.

Hitting an assert is much easier to debug than misbehavior or a crash.
Catching should-never-happens is a good thing.


Thanks,
pq

> 
> Cheers,
> 
> > ---
> >  libweston-desktop/xwayland.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
> > index f0cd1ed..7b8f6db 100644
> > --- a/libweston-desktop/xwayland.c
> > +++ b/libweston-desktop/xwayland.c
> > @@ -83,6 +83,8 @@ weston_desktop_xwayland_surface_change_state(struct weston_desktop_xwayland_surf
> >
> >  	if (surface->state != state) {
> >  		if (surface->state == XWAYLAND) {
> > +			assert(!surface->added);
> > +
> >  			weston_desktop_surface_unlink_view(surface->view);
> >  			weston_view_destroy(surface->view);
> >  			surface->view = NULL;
> > @@ -100,6 +102,8 @@ weston_desktop_xwayland_surface_change_state(struct weston_desktop_xwayland_surf
> >  		}
> >
> >  		if (state == XWAYLAND) {
> > +			assert(!surface->added);
> > +
> >  			surface->view =
> >  				weston_desktop_surface_create_view(surface->surface);
> >  			weston_layer_entry_insert(&surface->xwayland->layer.view_list,
> >  
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20161124/eaf979bc/attachment.sig>