[Spice-devel] [PATCH spice-server] smartcard: Optimise sending data

[Pavel Grunt]

On Fri, 2016-12-02 at 11:31 -0500, Frediano Ziglio wrote:
> > 
> > As data is packae in a single piece of memory send it
> > altogether.
> > 
> 
> typo: packaged
> 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/smartcard-channel-client.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/server/smartcard-channel-client.c
> > b/server/smartcard-channel-client.c
> > index 35327dc..95be310 100644
> > --- a/server/smartcard-channel-client.c
> > +++ b/server/smartcard-channel-client.c
> > @@ -212,10 +212,7 @@ void
> > smartcard_channel_client_send_data(RedChannelClient
> > *rcc,
> >      spice_assert(rcc);
> >      spice_assert(vheader);
> >      red_channel_client_init_send_data(rcc,
> > SPICE_MSG_SMARTCARD_DATA, item);
> > -    spice_marshaller_add_ref(m, (uint8_t*)vheader,
> > sizeof(VSCMsgHeader));
> > -    if (vheader->length > 0) {
> > -        spice_marshaller_add_ref(m, (uint8_t*)(vheader+1),
> > vheader->length);
> > -    }
> > +    spice_marshaller_add_ref(m, (uint8_t*)vheader,
> > sizeof(VSCMsgHeader) +
> > vheader->length);
> >  }
> >  
> >  void smartcard_channel_client_send_error(RedChannelClient *rcc,
> >  SpiceMarshaller *m, RedPipeItem *item)
> > --
> > 2.9.3
> > 
> > 
> 
> Note that vheader->length is unsigned.
> 
> So original code:
> 
>     spice_marshaller_add_ref(m, (uint8_t*)vheader,
> sizeof(VSCMsgHeader));
>     if (vheader->length > 0) {
>        spice_marshaller_add_ref(m, (uint8_t*)(vheader+1), vheader-
> >length);
>     }
> 
> can be written like
> 
>     if (vheader->length < 0) {
>        // IMPOSSIBLE !!
>     } else if (vheader->length == 0) {
>        spice_marshaller_add_ref(m, (uint8_t*)vheader,
> sizeof(VSCMsgHeader));
>     } else {
>        spice_marshaller_add_ref(m, (uint8_t*)vheader,
> sizeof(VSCMsgHeader));
>        spice_marshaller_add_ref(m, (uint8_t*)(vheader+1), vheader-
> >length);
>     }
> 
> note that vheader has type VSCMsgHeader so 
>   (uint8_t*)(vheader+1) == ((uint8_t*)vheader) +
> sizeof(VSCMsgHeader)
> which lead to
> 
>     if (vheader->length < 0) {
>        // IMPOSSIBLE !!
>     } else if (vheader->length == 0) {
>        spice_marshaller_add_ref(m, (uint8_t*)vheader,
> sizeof(VSCMsgHeader) + vheader->length);
>     } else {
>        spice_marshaller_add_ref(m, (uint8_t*)vheader,
> sizeof(VSCMsgHeader));
>        spice_marshaller_add_ref(m, ((uint8_t*)vheader) +
> sizeof(VSCMsgHeader), vheader->length);
>     }
> 
> that is
> 
>     if (vheader->length < 0) {
>        // IMPOSSIBLE !!
>     } else if (vheader->length == 0) {
>        spice_marshaller_add_ref(m, (uint8_t*)vheader,
> sizeof(VSCMsgHeader) + vheader->length);
>     } else {
>        spice_marshaller_add_ref(m, (uint8_t*)vheader,
> sizeof(VSCMsgHeader) + vheader->length);
>     }
> 
> that's 
> 
>     spice_marshaller_add_ref(m, (uint8_t*)vheader,
> sizeof(VSCMsgHeader) + vheader->length);
> 
> Frediano

Ack,
Pavel