On Wed, 25 Jan 2017 08:24:55 +0100 Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote: > On Tue, Jan 24, 2017 at 07:52:11PM +0100, Michal Suchánek wrote: > > On Tue, 24 Jan 2017 09:45:37 +0100 > > Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote: > > > > > > In this big switch, VD_AGENT_FILE_XFER_* and > > > VD_AGENT_CLIENT_DISCONNECTED were previously not subject to > > > a size check, VD_AGENT_DISPLAY_CONFIG and VD_AGENT_REPLY did not > > > appear in the switch, and > > > > They appear in the min_size array, however. So they should be > > handled. This is now a separate size check function that can be > > used to check check size of any message, even messages the > > dispatcher does not handle. > > > > Also the XFER messages need to be size checked later when they are > > swapped. Actually the fields of the messages are accessed already > > so it is an error to not check they are actually present. > > > > > VD_AGENT_CLIPBOARD_RELEASE/VD_AGENT_CLIPBOARD_RELEASE were doing > > > a < comparison, not a !=. I'm not necessarily opposed to these > > > changes, but I'd keep them for an additional commit, or at least > > > explain why this is ok to do in the commit log. > > > > This is definitely because they were lumped together with the > > variable length clipboard messages. > > Yeah, as I said, I don't think it's wrong to have them there, but I'd > add them in a separate message, so that there's a mostly mechanical > commit which adds the new function, but is strictly equivalent to the > old code in terms of what is tested and how it's tested. Then once you > have the function, you can refine things, add missing enum values, > refine the <= vs != tests, ... There was never a point when the code was equivalent after removing the size checks from the dispatch switch and putting them into a separate piece of code. There were unintentional errors in the first attempt taking out the pieces one by one and collecting them elsewhere. So here I just implement new separate check which is based on the headers which define the messages as variable or fixed size and remove the old checks which were based on historical evolution and were obviously more or less incorrect in places. There was never an intent to make to code bug to bug equivalent. Thanks Michal
Attachment:
pgpRLTGdqL5H3.pgp
Description: OpenPGP digital signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel