[PATCH wayland 2/3] Add version information to wl_message signatures.

[Bill Spitzak]

Jason Ekstrand wrote:

> That is exactly what this patch allows you to do.  More particularly, 
> this patch protects the compositor from bad clients that attempt to use 
> requests that are not supported by the compositor.  In the current 
> implementation, the compositor simply crashes.

I'm confused as to why crashes cannot be avoided in much simpler ways.

For instance if version 1 of FooObject acted like the "twiddle" setting 
was 5. Then a new version is made because it looked better to default 
"twiddle" to zero and add a "set_twiddle" message. I would expect the 
implementation in a compositor that understands the new version to be 
something like this:

     struct FooObject {
        int twiddle;
        int version;
     };

     FooObject* makeFooObject(int version) {
        x = new FooObject;
        x.twiddle = version < 2 ? 5 : 0;
        x.version = version;
     }

     // this is the function put in dispatch table:
     set_twiddle(FooObject* x, int value) {
        if (version < 2) error; // optional if the change is harmless
        x.twiddle = value;
     }

You seem to be writing it as though the compositor does this:

     struct FooObject1 {};

     struct FooObject2 { int twiddle; }

     FooObject* makeFooObject(int version) {
         if (version < 2) {
              x = new FooObject1;
              return x;
         } else {
              x = new FooObject2;
              x.twiddle = 0;
              return x;
         }
     }

     set_twiddle(void* x, int value) {
         ((FooObject2*)x)->twiddle = value; // crashes if it is a 
FooObject1!
     }

I find that unlikely. However even if there are different structures, if 
they share a "version" at the start the implementation can still check 
the version number like in my first example.

This also does not solve methods that have to act different depending on
the version. Which, if they are different structures, is true for all 
the methods that are in common. So adding "fail" as something it does 
depending on version seems easy.

>     To match your use of atoi it would be better to skip all the leading
>     digits (possibly in other code where you know you are at the start
>     of the signature), then use this old version of the code.
> 
> 
> I thought about doing that but this is less invasive. Also, I like this 
> method of counting better anyway because it's a bit more robust.  
> Suppose we add in some other type-specifier than "?".  In that case, the 
> old implementation wouldn't work anyway.

I am much more worried about a new argument type being added. This will 
miscount them unless it is rewritten.

Perhaps there could be rules about what counts as a type and what is a 
prefix. Maybe 'a'-'z' are types and all other characters prefix?