[Spice-devel] [spice 2/7] server: De-duplicate the DISPLAY_CLIENT_TIMEOUT definition

Frediano Ziglio fziglio at redhat.com
Fri Dec 11 07:59:18 PST 2015


> 
> On Fri, 11 Dec 2015, Frediano Ziglio wrote:
> 
> > > 
> > > Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
> > > ---
> > >  server/cursor-channel.c | 1 +
> > >  server/red-worker.h     | 1 -
> > >  2 files changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> > > index 840ff30..0b5ad75 100644
> > > --- a/server/cursor-channel.c
> > > +++ b/server/cursor-channel.c
> > > @@ -22,6 +22,7 @@
> > >  #include <glib.h>
> > >  #include "common/generated_server_marshallers.h"
> > >  #include "cursor-channel.h"
> > > +#include "dcc.h"
> > >  
> > >  #define CLIENT_CURSOR_CACHE_SIZE 256
> > >  
> > 
> > NACK this!
> > This is a wrong dependency.
> 
> Does this mean cursor-channel.c should include red-worker.h and
> DISPLAY_CLIENT_TIMEOUT should be removed from dcc.h?
> 
> Or should cursor-channel.c not use DISPLAY_CLIENT_TIMEOUT at all?
> 

I had a look at where is defined and used and why.
I think the constant name is quite misleading.
Is defined in dcc.h and red-worker.h and used for cursor and display
(in multiple files and functions).
IMHO this macro should be named just CLIENT_TIMEOUT, WORKER_CLIENT_TIMEOUT
or COMMON_CLIENT_TIMEOUT and surely defined in red-worker.h only.

> 
> > > diff --git a/server/red-worker.h b/server/red-worker.h
> > > index 44f35f7..f5e9d84 100644
> > > --- a/server/red-worker.h
> > > +++ b/server/red-worker.h
> > > @@ -33,7 +33,6 @@ typedef struct CommonChannelClient {
> > >  } CommonChannelClient;
> > >  
> > >  #define COMMON_CHANNEL_CLIENT(Client) ((CommonChannelClient*)(Client))
> > > -#define DISPLAY_CLIENT_TIMEOUT 30000000000ULL //nano
> > >  
> > >  #define CHANNEL_RECEIVE_BUF_SIZE 1024
> > >  typedef struct CommonChannel {
> > > --
> > > 2.6.2
> > 
> 
> 
> --
> Francois Gouget <fgouget at codeweavers.com>
> 

Frediano


More information about the Spice-devel mailing list