On Mon, Oct 05, 2015 at 12:04:49PM -0500, Derek Foreman wrote: > On 30/09/15 03:23 PM, Jasper St. Pierre wrote: > > I don't necessarily like this. The absence of a serial can have > > radical meanings on behavior. Being able to pass 0 to mean "no serial" > > anywhere we currently rely on a serial seems like poor design to me, > > and can easily be done by mistake. > > Eek, this wasn't my intention at all. I figured I'd have to add errors > to some existing protocol in the case that they were passed "invalid serial" > > There are cases in weston where it would be quite nice to have a > sentinel value to use instead of having to have a bool for "this serial > number is legit" too.
Even though probably unlikely, for clients unaware of a possible 0 == no serial, this would mean that they would suddenly start to be killed when when they before worked just fine. > > > In cases where we have two behaviors for serial-aware and > > non-serial-aware operations, I would rather have two different client > > requests. This would be my preference as well. Partly because the semantics of a request with a serial and one without will probably behave differently, and partly because the existing places where you pass a serial has no mentioning of any "non-serial" or "invalid serial" situations and we'd now just add a bunch of undefined behaviour. Is it really a big deal to have to multiple requests that do things differently? > > Fair enough. > > (there's still some text further down...) > > > On Wed, Sep 30, 2015 at 12:47 PM, Daniel Stone <dan...@fooishbar.org> wrote: > >> Hi, > >> > >> On 30 September 2015 at 16:59, Derek Foreman <der...@osg.samsung.com> > >> wrote: > >>> Having an invalid serial number is quite useful - for example, we can > >>> have protocol requests that optionally take a serial number or 0 > >>> instead of having two similar requests. > >>> > >>> Signed-off-by: Derek Foreman <der...@osg.samsung.com> > >>> --- > >>> > >>> Well, let's see where this goes ;) > >>> > >>> I've seen a few times now when having an invalid serial number would be > >>> helpful, is it too late to do this? Are we going to break anything? > >>> > >>> Anything currently doing math on serial numbers can be wrong by one if one > >>> of the serial numbers has wrapped around - this probably doesn't matter? > >>> > >>> I do wonder if we should have a wl_serial_compare() as part of wayland > >>> but the semantics are tricky (a big serial and a small serial may actually > >>> have happened quite close to eachother if the small happened after a > >>> wrap around). > >>> > >>> I think the usual comparisons of interest are equality and "is A less > >>> than B > >>> within (arbitrary distance to compensate for wrap-around)". I think > >>> strcmp() > >>> like semantics won't necessarily be good here because we don't usually > >>> need > >>> something to tell us "b happened before a", more "these almost certainly > >>> happened in the order specified in the function call". > >>> > >>> Where do we go from here? > >> > >> From the IRC discussion way back when: > >> 20:27 < krh> if we do need to make a new serial, I'd much prefer to > >> have it be 1 << 31 > >> 20:27 < krh> and then just restrict normal serial numbers space to 0 - > >> (1<<31 - 1) > >> 20:28 < krh> so that serial number increment is just serial = (serial > >> + 1) & 0x7fffffff > >> > >> I have no relevant opinion on the matter, but I guess that 0x80000000 > >> (or 0xffffffff) is a much more obviously invalid serial number than 0, > >> which could also just be the victim of something uninitialised. > > Yeah... I prefer 0 for just that reason - if someone's passing an > uninitialized serial they're buggy and it'd be nice to know about it (at > least in a log file or something - I'm not necessarily advocating a kill) > > Also, we get 0 for free from any of our zalloc helpers, which is nice if > we're using the invalid serial to mean "haven't received a valid serial > yet" or similar in weston. I think such scenario should be made more obvious in the client. A client that doesn't know whether it has a valid serial or not seems fragile. Jonas > > If we are going to make half the space invalid, we do have the option of > having an "invalid serial" and a "no serial" value - but this is > starting to make the serial feel a little too opaque/confusing I think? > > So, I guess now we have two problems: > > Do we even really want an "invalid serial"? > > What should it be? > > > I'm fine with the non-zero invalid serial if the first question can be > resolved... :) > > >> A helper would be nice, though like you say I think it has to be > >> directional/multi-return (older inc. wraparound? identical? > >> newer/error? invalid?). It'd probably be best to do that inside an > >> active user and only then shuffle it into Wayland. > >> > >> So, with the change to a different serial, and a note in documentation > >> somewhere that any extension relying on the invalid-serial behaviour > >> must itself explicitly document that: > >> Acked-by: Daniel Stone <dani...@collabora.com> > >> > >> Cheers, > >> Daniel > >> _______________________________________________ > >> wayland-devel mailing list > >> wayland-devel@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel > > > > > > > > _______________________________________________ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
