<div dir="ltr"><div><div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 12, 2014 at 1:34 PM, Jasper St. Pierre <span dir="ltr"><<a href="mailto:jstpierre@mecheye.net" target="_blank">jstpierre@mecheye.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>In the xdg-shell thread recently, Pekka had a lot of concerns of the kind "what happens when the client makes an unexpected or illegal request. Should it be an error?"<br>
</div></div></blockquote><div><br></div><div>Yes and no.  More on that in a line or two. <br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">
<div>I have an answer for this: Any behavior not defined is undefined behavior.  If it's not explicitly mentioned in the protocol, it's undefined behavior.<br>
</div><div><br></div><div>Undefined behavior means that anything can happen. The compositor can send you a fatal error. It can mess up your objects in undetectable ways. The surface contents you present might now be garbage. Obviously, compositors should try *very* hard to catch all edge cases and send protocol errors, but it might be difficult or impractical that we can catch all cases.<br>
</div></div></blockquote><div><br></div><div>I'm a little hesitant to above fully.  I would rather reserve the term "undefined behavior" to mean things that may not be what the client intends but which are not fatal.  That said, most things shouldn't be this kind of undefined behavior;  most client errors should be the kind that potentially lead to death.<br>
<br></div><div>I think the better thing to do would be to clarify the protocol to say how it should be used.  Any time a client does something that's outside of what the protocol says, it's liable to get killed.  We should be more vigilant when writing protocols to specify how it should properly be used so that we aren't left with strange ambiguity.  There a couple of reasons for this.<br>
<br></div><div>1) It will make better protocols.  If protocols are written as "this is what you should do" it forces the writer to think about all of the valid ways to use it.  Most likely, those are the only ways of using it that will be properly implemented in the compositor anyway, so clients doing anything else are liable to run into problems.<br>
<br></div><div>2) It's better for backwards-compatibility.  It's easier to define a certain behaviour in a later version if the result in earlier versions was "don't do this, or I'll kill your client".  If there were strange, poorly-defined, behaviors before, we have to keep those working in future versions.<br>
<br></div><div>3) If there is something that should result in non-fatal undefined behaviour, we want to identify and document it.  This way we make sure that compositors do something sensible and, at the very least, don't crash.<br>
</div><div><br></div><div>That said, I think it's better to clean up the protocol docs than to try to make some blanket statement about undefined behaviour.  Anywhere weston currently kills the client without it being particularly well-documented, I think we can feel free to put that in the "don't do this" category.  But I'd rather see us have those rules written down where they are relevant than have another nebulous statement to help us interpret the other nebulous statements.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>As a quick example, we have been discussing "surface roles" in the past week with xdg_surface. Unfortunately, Weston doesn't properly mark cursor or DND surfaces as having roles, and there's no easy way to check for it in the codebase.<br>

<br><div>From my reading of the code, it seems what would happen is that the cursor would disappear as the desktop-shell code changes the surface's position, and on the next mouse move, the cursor would reappear as a wl_pointer.enter event is sent out over the new surface.<br>
</div></div></div></blockquote><div><br></div><div>Deleting the cursor surface without wl_pointer.set_cursor(null) is something that should probably count as "undefined behavior" in the sense that it would be strange but not fatal.  For instance, the compositor could just hang on to the last buffer and keep using it.  There are all sorts of ways to gracefully handle that.  However, the client shouldn't expect anything in particular.  It could, for instance, result in no cursor.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div>
</div>Obviously, no valid program should ever try to reuse a cursor surface as an xdg_surface.<br></div></div></blockquote><div><br></div><div>Yeah, feel free to kill the client for that.  But we should add a line to wl_pointer that says something like "Once a surface has been used in set_cursor, it is considered a cursor surface and should never be used for anything other than set_cursor".<br>
<br></div><div>Make sense?<br></div><div>--Jason Ekstrand<br></div></div><br></div></div></div>