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

Submitted by Frediano Ziglio on Dec. 2, 2016, 4:18 p.m.

Details

Message ID 20161202161850.13170-1-fziglio@redhat.com
State Accepted
Commit b8f06e243790998bcb15054f39f29741f8d7bb12
Headers show
Series "smartcard: Optimise sending data" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Frediano Ziglio Dec. 2, 2016, 4:18 p.m.
As data is packae in a single piece of memory send it
altogether.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 server/smartcard-channel-client.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Patch hide | download patch | download mbox

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)

Comments

> 
> As data is packae in a single piece of memory send it
> altogether.
> 

typo: packaged

> Signed-off-by: Frediano Ziglio <fziglio@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
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@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