On Thu, Jan 26, 2017 at 09:37:56AM -0500, Frediano Ziglio wrote:
> >
> > On Thu, Jan 26, 2017 at 10:56:45AM +0000, Frediano Ziglio wrote:
> > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > > ---
> > > server/spicevmc.c | 10 +++++-----
> > > 1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/server/spicevmc.c b/server/spicevmc.c
> > > index a92ec7f..23e60b3 100644
> > > --- a/server/spicevmc.c
> > > +++ b/server/spicevmc.c
> > > @@ -585,12 +585,13 @@ static uint8_t
> > > *spicevmc_red_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,
> > > uint32_t size)
> > > {
> > > RedVmcChannel *channel;
> > > - RedClient *client = red_channel_client_get_client(rcc);
> > > -
> > > - channel = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
> > > + RedClient *client;
> > >
> > > switch (type) {
> > > case SPICE_MSGC_SPICEVMC_DATA:
> > > + client = red_channel_client_get_client(rcc);
> > > + channel = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
> > > +
> > > assert(!channel->recv_from_client_buf);
> > >
> > > channel->recv_from_client_buf =
> > > red_char_device_write_buffer_get(channel->chardev,
> > > @@ -615,10 +616,9 @@ static void
> > > spicevmc_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
> > > {
> > > RedVmcChannel *channel;
> > >
> > > - channel = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
> > > -
> > > switch (type) {
> > > case SPICE_MSGC_SPICEVMC_DATA:
> > > + channel = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
> > > /* buffer wasn't pushed to device */
> > > red_char_device_write_buffer_release(channel->chardev,
> > > &channel->recv_from_client_buf);
> > > break;
> >
> > This is something that I'd just let the compiler handle if needed. Note
> > that this also change some variables from being always initialized to
> > sometimes being initialized, which is in my opinion is not necessarily a
> > good move.
> >
> > Christophe
> >
>
> In this case the compiler cannot do it. How can it know that not calling
> a function does not make any difference?
So red_channel_client_get_channel() should be annotated with
G_GNUC_PURE so that the compiler knows about this and can optimize
everywhere by itself?
Thinking more about this patch, even though I hate the {} you have to
add, I'd be ok with
spicevmc_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
{
switch (type) {
case SPICE_MSGC_SPICEVMC_DATA: {
RedVmcChannel *channel;
channel = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
/* buffer wasn't pushed to device */
red_char_device_write_buffer_release(channel->chardev,
&channel->recv_from_client_buf);
break;
}
which improves the reading of the code in my opinion (you know 'channel'
is not going to be used anywhere else).
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel