library: Unify signal handlers

This commit drops quite a lot of repetition by moving switch
and add+remove handlers common parts to separate functions.

Change-Id: I80fc2d3ab6d7990e2633b2a3660ff3ba527ea558

diff --git a/libsessiond/src/lib.c b/libsessiond/src/lib.c
index 1782d23..28ebe13 100644 (file)
--- a/libsessiond/src/lib.c
+++ b/libsessiond/src/lib.c
@@ -112,7 +112,7 @@ static client_callbacks_data_t switch_user_completion_callbacks_data_mt = {
 };
 
 typedef struct {
-       void *client_callback;
+       subsession_event_callback client_callback;
        void *client_callback_data;
        GVariant *params;
        int signal_subscribed;
@@ -254,7 +254,7 @@ static int make_sure_connection_is_not_null()
        } \
 } while (0)
 
-static signal_client_data_t *make_new_signal_callback_client_data(void *client_callback, void *client_callback_data, GVariant *params) {
+static signal_client_data_t *make_new_signal_callback_client_data(subsession_event_callback client_callback, void *client_callback_data, GVariant *params) {
 
        if(params == NULL) {
                return NULL;
@@ -272,7 +272,7 @@ static signal_client_data_t *make_new_signal_callback_client_data(void *client_c
        return NULL;
 }
 
-static gint g_compare_session_uid_params (  gconstpointer client_data,   gconstpointer parameters) {
+static gint g_compare_session_uid_params(gconstpointer client_data, gconstpointer parameters) {
 
        maybe_not_comparable_if(
                client_data_are_null(client_data,
@@ -417,52 +417,66 @@ static signal_client_data_t *find_in_callbacks_data( client_callbacks_data_t *cl
        return signal_data;
 }
 
-static void signal_add_user_started_handler(GDBusConnection *connection,
-       const gchar *sender_name,
-       const gchar *object_path,
-       const gchar *interface_name,
-       const gchar *signal_name,
-       GVariant *parameters,
-       gpointer client_data) {
+static bool signal_handler_internal(const gchar *signal_name,
+                                   GVariant *parameters,
+                                   gpointer client_data,
+                                   GCompareFunc compare_fn,
+                                   signal_client_data_t **signal_client_data)
+{
+       assert(client_data);
 
-       if (parameters == NULL || client_data == NULL) {
-               LOGE("parameters: (%s), client_data: (%s)", parameters == NULL ? "NULL" : "OK", client_data == NULL ? "NULL" : "OK");
+       if (parameters == NULL) {
+               LOGE("Signal %s invoked without parameters - ignoring", signal_name);
+               return false;
        }
 
        client_callbacks_data_t *client_callbacks_data = (client_callbacks_data_t *)client_data;
+       signal_client_data_t *signal_data = find_in_callbacks_data(client_callbacks_data, parameters, compare_fn);
 
-       if(client_callbacks_data->list == NULL) {
-               LOGE("list is NULL");
-               return;
+       if (signal_data == NULL || signal_data->client_callback == NULL ) {
+               LOGW("Unable to find handler matching provided parameters for %s signal", signal_name);
+               return false;
        }
 
-       signal_client_data_t *signal_data = find_in_callbacks_data(client_callbacks_data, parameters, g_compare_session_uid_params_wait);
-
-       if(signal_data == NULL || signal_data->client_callback == NULL ) {
-               LOGE("signal data: (%s),  callback (%s)", signal_data == NULL ? "NULL" : "not NULL",  signal_data == NULL ? "NULL" : signal_data->client_callback == NULL ? "NULL" : "not NULL");
-               return ;
-       }
+       *signal_client_data = signal_data;
+       return true;
+}
 
-       subsession_event_info event_info;
+static inline void *subsession_user_copy(subsession_user_t dst, subsession_user_t src)
+{
+       return memcpy(dst, src, strnlen(dst, sizeof(subsession_user_t)-1) + 1);
+}
 
-       event_info.event = SUBSESSION_ADD_USER_WAIT;
+static void signal_addremove_common_handler(GDBusConnection *connection,
+       const gchar *sender_name,
+       const gchar *object_path,
+       const gchar *interface_name,
+       const gchar *signal_name,
+       GVariant *parameters,
+       gpointer client_data,
+       subsession_event_type_e event)
+{
+       assert(event == SUBSESSION_ADD_USER_WAIT || event == SUBSESSION_REMOVE_USER_WAIT);
 
-       char *user_id = NULL;
+       signal_client_data_t *signal_data = NULL;
+       if (!signal_handler_internal(signal_name, parameters, client_data, g_compare_session_uid_params_wait, &signal_data))
+               return;
 
+       subsession_event_info event_info = { .event = event };
+       g_autofree char *user_id = NULL;
        maybe_g_variant_get_or_ret_(parameters, "(is)", &event_info.session_uid, &user_id);
 
-       if (error_on_bad_user_id(user_id)) {
-               g_free(user_id);
+       if (error_on_bad_user_id(user_id))
                return;
-       }
-       memcpy(event_info.add_user.user, user_id, strnlen(user_id, SUBSESSION_USER_MAXLEN)+1);
-       g_free(user_id);
 
-       subsession_event_callback event_callback = signal_data->client_callback;
-       event_callback(event_info, signal_data->client_callback_data);
+       // note assert() at the top of this function
+       subsession_user_copy(event == SUBSESSION_ADD_USER_WAIT ? event_info.add_user.user : event_info.remove_user.user, user_id);
+
+       assert(signal_data->client_callback);
+       signal_data->client_callback(event_info, signal_data->client_callback_data);
 }
 
-static void signal_remove_user_started_handler(GDBusConnection *connection,
+static void signal_add_user_started_handler(GDBusConnection *connection,
        const gchar *sender_name,
        const gchar *object_path,
        const gchar *interface_name,
@@ -470,41 +484,46 @@ static void signal_remove_user_started_handler(GDBusConnection *connection,
        GVariant *parameters,
        gpointer client_data)
 {
-       if(parameters == NULL || client_data == NULL) {
-               LOGE("arameters: (%s), client data: (%s)", parameters == NULL ? "NULL" : "not NULL", client_data == NULL ? "NULL" : "not NULL");
-               return;
-       }
+       signal_addremove_common_handler(connection, sender_name, object_path, interface_name, signal_name, parameters, client_data, SUBSESSION_ADD_USER_WAIT);
+}
 
-       client_callbacks_data_t *client_callbacks_data = (client_callbacks_data_t *)client_data;
+static void signal_remove_user_started_handler(GDBusConnection *connection,
+       const gchar *sender_name,
+       const gchar *object_path,
+       const gchar *interface_name,
+       const gchar *signal_name,
+       GVariant *parameters,
+       gpointer client_data)
+{
+       signal_addremove_common_handler(connection, sender_name, object_path, interface_name, signal_name, parameters, client_data, SUBSESSION_REMOVE_USER_WAIT);
+}
 
-       if(client_callbacks_data->list == NULL) {
-               LOGE("client_callbacks_data->list is NULL");
+static void signal_switch_user_common_handler(GDBusConnection *connection,
+       const gchar *sender_name,
+       const gchar *object_path,
+       const gchar *interface_name,
+       const gchar *signal_name,
+       GVariant *parameters,
+       gpointer client_data,
+       subsession_event_type_e event)
+{
+       signal_client_data_t *signal_data = NULL;
+       if (!signal_handler_internal(signal_name, parameters, client_data, g_compare_session_uid_params_switch_wait, &signal_data))
                return;
-       }
-
-       signal_client_data_t *signal_data = find_in_callbacks_data(client_callbacks_data, parameters, g_compare_session_uid_params_wait);
-
-       if(signal_data == NULL || signal_data->client_callback == NULL ) {
-               LOGE("signal data: (%s),  callback (%s)", signal_data == NULL ? "NULL" : "not NULL",  signal_data == NULL ? "NULL" : signal_data->client_callback == NULL ? "NULL" : "not NULL");
-               return ;
-       }
-
-       subsession_event_info event_info;
 
-       event_info.event = SUBSESSION_REMOVE_USER_WAIT;
+       subsession_event_info event_info = { .event = event };
+       g_autofree char *prev_user = NULL;
+       g_autofree char *next_user = NULL;
+       maybe_g_variant_get_or_ret_(parameters, "(ixss)", &event_info.session_uid, &event_info.switch_user.switch_id, &prev_user, &next_user);
 
-       char *user_id = NULL;
-       maybe_g_variant_get_or_ret_(parameters, "(is)", &event_info.session_uid, &user_id);
-
-       if (error_on_bad_user_id(user_id)) {
-               g_free(user_id);
+       if (error_on_switch_bad_user_id(prev_user) || error_on_switch_bad_user_id(next_user))
                return;
-       }
-       memcpy(event_info.remove_user.user, user_id, strnlen(user_id, SUBSESSION_USER_MAXLEN)+1);
-       g_free(user_id);
 
-       subsession_event_callback event_callback = signal_data->client_callback;
-       event_callback(event_info, signal_data->client_callback_data);
+       subsession_user_copy(event_info.switch_user.prev_user, prev_user);
+       subsession_user_copy(event_info.switch_user.next_user, next_user);
+
+       assert(signal_data->client_callback);
+       signal_data->client_callback(event_info, signal_data->client_callback_data);
 }
 
 static void signal_switch_user_started_handler(GDBusConnection *connection,
@@ -515,49 +534,10 @@ static void signal_switch_user_started_handler(GDBusConnection *connection,
        GVariant *parameters,
        gpointer client_data)
 {
-       if (parameters == NULL || client_data == NULL) {
-               LOGE("parameters: (%s) client_data: (%s)", parameters == NULL ? "NULL" : "not NULL", client_data == NULL ? "NULL" : "not NULL");
-               return;
-       }
-
-       client_callbacks_data_t *client_callbacks_data = (client_callbacks_data_t *)client_data;
-
-       signal_client_data_t *signal_data = find_in_callbacks_data(client_callbacks_data, parameters, g_compare_session_uid_params_switch_wait);
-
-       if(signal_data == NULL || signal_data->client_callback == NULL ) {
-               LOGE("signal data: (%s),  callback (%s)", signal_data == NULL ? "NULL" : "not NULL",  signal_data == NULL ? "NULL" : signal_data->client_callback == NULL ? "NULL" : "not NULL");
-               return ;
-       }
-       subsession_event_info event_info = {0};
-
-       char *prev_user = NULL;
-       char *next_user = NULL;
-
-       event_info.event = SUBSESSION_SWITCH_USER_WAIT;
-       maybe_g_variant_get_or_ret_(parameters, "(ixss)",
-               &event_info.session_uid,
-               &event_info.switch_user.switch_id,
-               &prev_user,
-               &next_user);
-
-       if (error_on_switch_bad_user_id( prev_user) || error_on_switch_bad_user_id(next_user)) {
-               g_free(prev_user);
-               g_free(next_user);
-               return;
-       }
-
-       memcpy(event_info.switch_user.prev_user, prev_user, strnlen(prev_user, SUBSESSION_USER_MAXLEN)+1);
-       memcpy(event_info.switch_user.next_user, next_user, strnlen(next_user, SUBSESSION_USER_MAXLEN)+1);
-
-       g_free(prev_user);
-       g_free(next_user);
-
-       subsession_event_callback event_callback = signal_data->client_callback;
-
-       event_callback(event_info, signal_data->client_callback_data);
+       signal_switch_user_common_handler(connection, sender_name, object_path, interface_name, signal_name, parameters, client_data, SUBSESSION_SWITCH_USER_WAIT);
 }
 
-static void  signal_switch_user_completion_handler( GDBusConnection *connection,
+static void signal_switch_user_completion_handler(GDBusConnection *connection,
        const gchar *sender_name,
        const gchar *object_path,
        const gchar *interface_name,
@@ -565,51 +545,7 @@ static void  signal_switch_user_completion_handler( GDBusConnection *connection,
        GVariant *parameters,
        gpointer client_data)
 {
-       if (parameters == NULL) {
-               LOGE("parameters is NULL");
-               return;
-       }
-
-       client_callbacks_data_t *client_callbacks_data = client_data;
-
-       if(client_data == NULL || client_callbacks_data->list == NULL) {
-               LOGE("client data: (%s),  client callbacks data list (%s)", client_data == NULL ? "NULL" : "not NULL", client_callbacks_data->list == NULL ? "NULL" : "not NULL");
-               return;
-       }
-
-       signal_client_data_t *signal_data = find_in_callbacks_data(client_callbacks_data, parameters, g_compare_session_uid_params_switch_wait);
-
-       if(signal_data == NULL || signal_data->client_callback == NULL) {
-               LOGE("signal data: (%s),  callback (%s)", signal_data == NULL ? "NULL" : "not NULL",  signal_data == NULL ? "NULL" : signal_data->client_callback == NULL ? "NULL" : "not NULL");
-               return ;
-       }
-
-       char *prev_user = NULL;
-       char *next_user = NULL;
-
-       subsession_event_info event_info = {0};
-       event_info.event = SUBSESSION_SWITCH_USER_COMPLETION;
-
-       maybe_g_variant_get_or_ret_(parameters, "(ixss)",
-               &event_info.session_uid,
-               &event_info.switch_user.switch_id,
-               &prev_user,
-               &next_user);
-
-       if (error_on_switch_bad_user_id( prev_user) || error_on_switch_bad_user_id(next_user)) {
-               g_free(prev_user);
-               g_free(next_user);
-               return;
-       }
-
-       memcpy(event_info.switch_user.prev_user, prev_user, strnlen(prev_user, SUBSESSION_USER_MAXLEN)+1);
-       memcpy(event_info.switch_user.next_user, next_user, strnlen(next_user, SUBSESSION_USER_MAXLEN)+1);
-
-       g_free(prev_user);
-       g_free(next_user);
-
-       subsession_event_callback event_callback = signal_data->client_callback;
-       event_callback(event_info, signal_data->client_callback_data);
+       signal_switch_user_common_handler(connection, sender_name, object_path, interface_name, signal_name, parameters, client_data, SUBSESSION_SWITCH_USER_COMPLETION);
 }
 
 #define subscribe_cfg_(_signal_name)           \