[Spice-devel] [PATCH spice-common 1/6] messages: Remove fields not used by the protocol

Frediano Ziglio fziglio at redhat.com
Sat Feb 23 18:19:23 UTC 2019


> On Thu, 2019-02-21 at 10:38 +0000, Frediano Ziglio wrote:
> > These fields are not used by the protocol.
> > Avoid spice-gtk and spice-server to use them by mistake.
> > This can cause memory errors (data_size is not used or
> > is not set correctly) and useless code (spice-gtk uses
> > the pub_key* fields but these fields are not sent to
> > the server as the protocol does not have them).
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  common/messages.h | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/common/messages.h b/common/messages.h
> > index 3e37235..91ac7ad 100644
> > --- a/common/messages.h
> > +++ b/common/messages.h
> > @@ -43,7 +43,6 @@
> >  SPICE_BEGIN_DECLS
> >  
> >  typedef struct SpiceMsgData {
> > -    uint32_t data_size;
> >      uint8_t data[0];
> >  } SpiceMsgData;
> >  
> > @@ -75,9 +74,6 @@ typedef struct SpiceMigrationDstInfo {
> >      uint16_t sport;
> >      uint32_t host_size;
> >      uint8_t *host_data;
> > -    uint16_t pub_key_type;
> > -    uint32_t pub_key_size;
> > -    uint8_t *pub_key_data;
> >      uint32_t cert_subject_size;
> >      uint8_t *cert_subject_data;
> >  } SpiceMigrationDstInfo;
> 
> 
> Hmm, slightly tricky, this one. It's technically an ABI change, but
> since we only use spice-common as a submodule, I suppose it's OK to
> remove.
> 
> I'd like to see the commit log mention that this is leftover from v1 of
> the protocol, which we removed support for already.
> 

In reality is a bit more complicated, see 
https://lists.freedesktop.org/archives/spice-devel/2019-February/048089.html
and
https://lists.freedesktop.org/archives/spice-devel/2019-February/048126.html

Looking at spice-gtk code looks like that was changed from 2.0 to 2.1
protocol.
The change broke the protocol changing a message. As we used the new one
for more than 7 years removing the old one seems correct.
Note that the fields are only use in the client which will see them empty
(the message is from the server, the demarshaller don't know these fields
so cannot write them).

Any suggestion for improving the commit message?

Frediano


More information about the Spice-devel mailing list