[PATCH wayland] server: Make serial number 0 mean invalid

Thu Oct 8 00:27:18 PDT 2015

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 <daniel at fooishbar.org> wrote:
> >> Hi,
> >>
> >> On 30 September 2015 at 16:59, Derek Foreman <derekf at 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 <derekf at 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 <daniels at collabora.com>
> >>
> >> Cheers,
> >> Daniel
> >> _______________________________________________
> >> wayland-devel mailing list
> >> wayland-devel at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > 
> > 
> > 
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel