[Spice-devel] [PATCH 11/16] Fix crash when checking mouse mode

[Frediano Ziglio]

> 
> On Wed, 2016-01-27 at 12:48 +0000, Frediano Ziglio wrote:
> > From: Jonathon Jongsma <jjongsma at redhat.com>
> > 
> > Since the mouse mode is now stored in the inputs channel, we were
> > crashing when somebody was calling this API before the inputs channel
> > was created.
> 
> The same apply for more functions in inputs-channel.h, no ?
> 
> This patch should be squashed into / pushed before the "Remove global
> inputs channel"
> 

Being a fix of a not applied patch I would squash in it.

> > ---
> >  server/inputs-channel.c | 1 +
> >  server/reds.c           | 4 +++-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> > index 9f33f0d..e28b4b0 100644
> > --- a/server/inputs-channel.c
> > +++ b/server/inputs-channel.c
> > @@ -704,6 +704,7 @@ int inputs_channel_set_tablet(InputsChannel
> > *inputs, SpiceTabletInstance *tablet
> >  
> >  int inputs_channel_has_tablet(InputsChannel *inputs)
> >  {
> > +    spice_return_val_if_fail(inputs != NULL, FALSE);
> >      return !!inputs->tablet;
> >  }
> 
> Another option is to consider inputs == NULL as a valid state and
> return FALSE - looking below it is a valid state
> 
> int inputs_channel_has_tablet(InputsChannel *inputs)
> {
>   return inputs != NULL && !!inputs->tablet;
> }
> 

Here I would use same test style like

   return inputs != NULL && inputs->tablet != NULL;

I think is also more coherent with the rest of the code style.

Pavel, can you then post a patch of the other patch with
this patch squashed into it and fixed?

Frediano

> >  
> > diff --git a/server/reds.c b/server/reds.c
> > index f35b602..3f2a132 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -613,7 +613,9 @@ static void reds_update_mouse_mode(RedsState
> > *reds)
> >      int allowed = 0;
> >      int qxl_count = red_dispatcher_qxl_count();
> >  
> > -    if ((agent_mouse && vdagent) || (inputs_channel_has_tablet(reds-
> > >inputs_channel) && qxl_count == 1)) {
> > +    if ((agent_mouse && vdagent) ||
> > +        ((reds->inputs_channel && inputs_channel_has_tablet(reds-
> > >inputs_channel)) &&
> > +         qxl_count == 1)) {
> then you would not need to check if the reds->inputs_channel is NULL
> 
> Pavel
> >          allowed = reds->dispatcher_allows_client_mouse;
> >      }
> >      if (allowed == reds->is_client_mouse_allowed) {
>