[Spice-devel] [vdagent-linux v2 6/6] console-kit: include handler for dbus match

[Victor Toso]

On Wed, Apr 27, 2016 at 03:54:16PM -0500, Jonathon Jongsma wrote:
> On Sat, 2016-04-23 at 12:27 +0200, Victor Toso wrote:
> > At the moment we request to read signals on ConsoleKit.Seat interface but
> > later
> > on we will request to read signals on different interface so moving this to
> > its
> > own function helps.
> > ---
> >  src/console-kit.c | 65 +++++++++++++++++++++++++++++++++++++++++++-----------
> > -
> >  1 file changed, 51 insertions(+), 14 deletions(-)
> > 
> > diff --git a/src/console-kit.c b/src/console-kit.c
> > index 41a1cfe..f3c4aa3 100644
> > --- a/src/console-kit.c
> > +++ b/src/console-kit.c
> > @@ -34,6 +34,7 @@ struct session_info {
> >      char *seat;
> >      char *active_session;
> >      int verbose;
> > +    gchar *match_seat_signals;
> >  };
> >  
> >  #define INTERFACE_CONSOLE_KIT "org.freedesktop.ConsoleKit"
> > @@ -49,6 +50,54 @@ struct session_info {
> >  static char *console_kit_get_first_seat(struct session_info *si);
> >  static char *console_kit_check_active_session_change(struct session_info
> > *si);
> >  
> > +static void si_dbus_match_remove(struct session_info *si)
> > +{
> > +    DBusError error;
> > +    if (si->match_seat_signals != NULL) {
> > +        dbus_error_init(&error);
> > +        dbus_bus_remove_match(si->connection,
> > +                              si->match_seat_signals,
> > +                              &error);
> > +        if (si->verbose)
> > +            syslog(LOG_DEBUG, "(console-kit) seat match removed: %s",
> > +                   si->match_seat_signals);
> > +        g_free(si->match_seat_signals);
> 
> Since we're using the NULL-ness of si->match_seat_signals to make decisions (see
> above), we should probably set it to NULL after we free it.
>

Agreed!

> > +    }
> > +}
> > +
> > +static void si_dbus_match_rule_update(struct session_info *si)
> > +{
> > +    DBusError error;
> > +
> > +    if (si->connection == NULL)
> > +        return;
> > +
> > +    si_dbus_match_remove(si);
> > +
> > +    /* Seat signals */
> > +    if (si->seat != NULL) {
> > +        si->match_seat_signals =
> > +            g_strdup_printf ("type='signal',interface='%s',path='%s',"
> > +                             "member='ActiveSessionChanged'",
> > +                             INTERFACE_CK_SEAT,
> > +                             si->seat);
> > +        if (si->verbose)
> > +            syslog(LOG_DEBUG, "(console-kit) seat match: %s",
> > +                   si->match_seat_signals);
> > +
> > +        dbus_error_init(&error);
> > +        dbus_bus_add_match(si->connection,
> > +                           si->match_seat_signals,
> > +                           &error);
> > +        if (dbus_error_is_set(&error)) {
> > +            syslog(LOG_WARNING, "Unable to add dbus rule match: %s",
> > +                   error.message);
> > +            dbus_error_free(&error);
> > +            g_free(si->match_seat_signals);
> > +        }
> > +    }
> > +}
> > +
> >  static void
> >  si_dbus_read_signals(struct session_info *si)
> >  {
> > @@ -106,7 +155,6 @@ struct session_info *session_info_create(int verbose)
> >  {
> >      struct session_info *si;
> >      DBusError error;
> > -    char match[1024];
> >  
> >      si = calloc(1, sizeof(*si));
> >      if (!si)
> > @@ -138,19 +186,7 @@ struct session_info *session_info_create(int verbose)
> >          return NULL;
> >      }
> >  
> > -    /* Register for active session changes */
> > -    snprintf(match, sizeof(match),
> > -             "type='signal',interface='%s',"
> > -             "path='%s',member='ActiveSessionChanged'",
> > -             INTERFACE_CK_SEAT, si->seat);
> > -    dbus_error_init(&error);
> > -    dbus_bus_add_match(si->connection, match, &error);
> > -    if (dbus_error_is_set(&error)) {
> > -        syslog(LOG_ERR, "Match Error (%s)", error.message);
> > -        session_info_destroy(si);
> > -        return NULL;
> > -    }
> > -
> > +    si_dbus_match_rule_update(si);
> >      return si;
> >  }
> >  
> > @@ -159,6 +195,7 @@ void session_info_destroy(struct session_info *si)
> >      if (!si)
> >          return;
> >  
> > +    si_dbus_match_remove(si);
> >      dbus_connection_close(si->connection);
> >      free(si->seat);
> >      free(si->active_session);
> 
> Otherwise looks OK to me
> 
> Acked-by: Jonathon Jongsma <jjongsma at redhat.com>