> > On Mon, Sep 04, 2017 at 10:47:45AM -0400, Frediano Ziglio wrote: > > > > > > On Fri, Sep 01, 2017 at 10:36:58AM +0100, Frediano Ziglio wrote: > > > > These constants are saved into record files so their values should not > > > > be changed in order to be able to use old record files. > > > > Recently 2 different patches has attempted to change these values > > > > without noticing the command above the enumeration. > > > > A runtime error will occur registering duplicate message numbers as > > > > soon as a display is created inside the VM. > > > > > > I would add a big /* INSERT NEW MESSAGES HERE */ right before > > > RED_WORKER_MESSAGE_COUNT, I *think* I'm much more likely to > > > be looking at the end of the enum rather than at its beginning when > > > looking where to add a new member. > > > > > > Why not use > > > verify(RED_WORKER_MESSAGE_DISPLAY_CONNECT == 5); > > > verify(RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC == 29); > > > to get compile-time failures if these get moved by mistake? > > > > > > Christophe > > > > > > > Would make sense. However I realized that this is not enough. > > > > Let say that I record the event 20 which is not handled (now) > > by the replay. I though: well, I can reuse this value. > > But consider if I want to try the same file with a new version. > > Maybe now the event 20, which does another thing, is now handled > > by the replay so the old file may stop working. Which is > > something I don't want as I want to continue to use the file > > for testing. So I would have to fix all the numbers, used or > > not by the replay. Which is what the comment said, but not > > what this patch is doing. > > On the other hand recording is not the main aim of these messages. > > And having holes in the enumeration in the long run would > > make the handler array bigger while maybe for cache efficiency > > would be better to have it contiguous. > > But maybe here I'm a bit too paranoid. > > We could save names instead of numbers or define different > > fixed enumeration for record/replay. > > Yes, this sounds better. The enum used for these dispatcher messages is > imo an internal implementation detail, the recording format is kind of > turning it into an ABI of some sort, which I don't think is good. > > Christophe > I already regret my suggestion :-) Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel