From ed3a9f6a30958ef90a24bc60aec86493974101d3 Mon Sep 17 00:00:00 2001

From: David Herrmann <dh.herrmann@gmail.com>

Date: Mon, 22 Sep 2014 17:55:31 +0200

Subject: [PATCH] terminal: handle callback errors in sysview instead of

 propagating them

We cannot sanely propagate error codes if we call into user-callbacks

multiple times for multiple objects. There is no way to merge those errors

or somehow propagate them.

However, we can just act similar to sd-event and print a log-message while

discarding the values. This way, we allow error-returns, but can properly

continue working on our objects.

---

 src/libsystemd-terminal/sysview.c | 175 +++++++++++++++++---------------------

 1 file changed, 76 insertions(+), 99 deletions(-)

diff --git a/src/libsystemd-terminal/sysview.c b/src/libsystemd-terminal/sysview.c

index 208c6ce25c..969514ad9d 100644

--- a/src/libsystemd-terminal/sysview.c

+++ b/src/libsystemd-terminal/sysview.c

@@ -272,7 +272,7 @@ static int session_take_control_fn(sd_bus *bus,

                                    void *userdata,

                                    sd_bus_error *ret_error) {

         sysview_session *session = userdata;

-        int error;

+        int r, error;

         session->slot_take_control = sd_bus_slot_unref(session->slot_take_control);

@@ -287,7 +287,12 @@ static int session_take_control_fn(sd_bus *bus,

                 error = 0;

         }

-        return context_raise_session_control(session->seat->context, session, error);

+        r = context_raise_session_control(session->seat->context, session, error);

+        if (r < 0)

+                log_debug("sysview: callback failed while signalling session control '%d' on session '%s': %s",

+                          error, session->name, strerror(-r));

+

+        return 0;

 }

 int sysview_session_take_control(sysview_session *session) {

@@ -548,10 +553,10 @@ static int context_raise_session_refresh(sysview_context *c, sysview_session *se

         return context_raise(c, &event, 0);

 }

-static int context_add_device(sysview_context *c, sysview_device *device) {

+static void context_add_device(sysview_context *c, sysview_device *device) {

         sysview_session *session;

-        int r, error = 0;

         Iterator i;

+        int r;

         assert(c);

         assert(device);

@@ -564,20 +569,16 @@ static int context_add_device(sysview_context *c, sysview_device *device) {

                         continue;

                 r = context_raise_session_attach(c, session, device);

-                if (r != 0)

-                        error = r;

+                if (r < 0)

+                        log_debug("sysview: callback failed while attaching device '%s' to session '%s': %s",

+                                  device->name, session->name, strerror(-r));

         }

-

-        if (error < 0)

-                log_debug("sysview: error while adding device '%s': %s",

-                          device->name, strerror(-r));

-        return error;

 }

-static int context_remove_device(sysview_context *c, sysview_device *device) {

+static void context_remove_device(sysview_context *c, sysview_device *device) {

         sysview_session *session;

-        int r, error = 0;

         Iterator i;

+        int r;

         assert(c);

         assert(device);

@@ -589,21 +590,18 @@ static int context_remove_device(sysview_context *c, sysview_device *device) {

                         continue;

                 r = context_raise_session_detach(c, session, device);

-                if (r != 0)

-                        error = r;

+                if (r < 0)

+                        log_debug("sysview: callback failed while detaching device '%s' from session '%s': %s",

+                                  device->name, session->name, strerror(-r));

         }

-        if (error < 0)

-                log_debug("sysview: error while removing device '%s': %s",

-                          device->name, strerror(-r));

         sysview_device_free(device);

-        return error;

 }

-static int context_change_device(sysview_context *c, sysview_device *device, struct udev_device *ud) {

+static void context_change_device(sysview_context *c, sysview_device *device, struct udev_device *ud) {

         sysview_session *session;

-        int r, error = 0;

         Iterator i;

+        int r;

         assert(c);

         assert(device);

@@ -615,21 +613,17 @@ static int context_change_device(sysview_context *c, sysview_device *device, str

                         continue;

                 r = context_raise_session_refresh(c, session, device, ud);

-                if (r != 0)

-                        error = r;

+                if (r < 0)

+                        log_debug("sysview: callback failed while changing device '%s' on session '%s': %s",

+                                  device->name, session->name, strerror(-r));

         }

-

-        if (error < 0)

-                log_debug("sysview: error while changing device '%s': %s",

-                          device->name, strerror(-r));

-        return error;

 }

-static int context_add_session(sysview_context *c, sysview_seat *seat, const char *id) {

+static void context_add_session(sysview_context *c, sysview_seat *seat, const char *id) {

         sysview_session *session;

         sysview_device *device;

-        int r, error = 0;

         Iterator i;

+        int r;

         assert(c);

         assert(seat);

@@ -637,7 +631,7 @@ static int context_add_session(sysview_context *c, sysview_seat *seat, const cha

         session = sysview_find_session(c, id);

         if (session)

-                return 0;

+                return;

         log_debug("sysview: add session '%s' on seat '%s'", id, seat->name);

@@ -654,35 +648,33 @@ static int context_add_session(sysview_context *c, sysview_seat *seat, const cha

         if (seat->public) {

                 session->public = true;

                 r = context_raise_session_add(c, session);

-                if (r != 0) {

+                if (r < 0) {

+                        log_debug("sysview: callback failed while adding session '%s': %s",

+                                  session->name, strerror(-r));

                         session->public = false;

                         goto error;

                 }

                 HASHMAP_FOREACH(device, seat->device_map, i) {

                         r = context_raise_session_attach(c, session, device);

-                        if (r != 0)

-                                error = r;

+                        if (r < 0)

+                                log_debug("sysview: callback failed while attaching device '%s' to new session '%s': %s",

+                                          device->name, session->name, strerror(-r));

                 }

-

-                r = error;

-                if (r != 0)

-                        goto error;

         }

-        return 0;

+        return;

 error:

         if (r < 0)

                 log_debug("sysview: error while adding session '%s': %s",

                           id, strerror(-r));

-        return r;

 }

-static int context_remove_session(sysview_context *c, sysview_session *session) {

+static void context_remove_session(sysview_context *c, sysview_session *session) {

         sysview_device *device;

-        int r, error = 0;

         Iterator i;

+        int r;

         assert(c);

         assert(session);

@@ -692,27 +684,25 @@ static int context_remove_session(sysview_context *c, sysview_session *session)

         if (session->public) {

                 HASHMAP_FOREACH(device, session->seat->device_map, i) {

                         r = context_raise_session_detach(c, session, device);

-                        if (r != 0)

-                                error = r;

+                        if (r < 0)

+                                log_debug("sysview: callback failed while detaching device '%s' from old session '%s': %s",

+                                          device->name, session->name, strerror(-r));

                 }

                 session->public = false;

                 r = context_raise_session_remove(c, session);

-                if (r != 0)

-                        error = r;

+                if (r < 0)

+                        log_debug("sysview: callback failed while removing session '%s': %s",

+                                  session->name, strerror(-r));

         }

         if (!session->custom)

                 sysview_session_release_control(session);

-        if (error < 0)

-                log_debug("sysview: error while removing session '%s': %s",

-                          session->name, strerror(-error));

         sysview_session_free(session);

-        return error;

 }

-static int context_add_seat(sysview_context *c, const char *id) {

+static void context_add_seat(sysview_context *c, const char *id) {

         sysview_seat *seat;

         int r;

@@ -721,7 +711,7 @@ static int context_add_seat(sysview_context *c, const char *id) {

         seat = sysview_find_seat(c, id);

         if (seat)

-                return 0;

+                return;

         log_debug("sysview: add seat '%s'", id);

@@ -731,54 +721,45 @@ static int context_add_seat(sysview_context *c, const char *id) {

         seat->public = true;

         r = context_raise_seat_add(c, seat);

-        if (r != 0) {

+        if (r < 0) {

+                log_debug("sysview: callback failed while adding seat '%s': %s",

+                          seat->name, strerror(-r));

                 seat->public = false;

-                goto error;

         }

-        return 0;

+        return;

 error:

         if (r < 0)

                 log_debug("sysview: error while adding seat '%s': %s",

                           id, strerror(-r));

-        return r;

 }

-static int context_remove_seat(sysview_context *c, sysview_seat *seat) {

+static void context_remove_seat(sysview_context *c, sysview_seat *seat) {

         sysview_session *session;

         sysview_device *device;

-        int r, error = 0;

+        int r;

         assert(c);

         assert(seat);

         log_debug("sysview: remove seat '%s'", seat->name);

-        while ((device = hashmap_first(seat->device_map))) {

-                r = context_remove_device(c, device);

-                if (r != 0)

-                        error = r;

-        }

+        while ((device = hashmap_first(seat->device_map)))

+                context_remove_device(c, device);

-        while ((session = hashmap_first(seat->session_map))) {

-                r = context_remove_session(c, session);

-                if (r != 0)

-                        error = r;

-        }

+        while ((session = hashmap_first(seat->session_map)))

+                context_remove_session(c, session);

         if (seat->public) {

                 seat->public = false;

                 r = context_raise_seat_remove(c, seat);

-                if (r != 0)

-                        error = r;

+                if (r < 0)

+                        log_debug("sysview: callback failed while removing seat '%s': %s",

+                                  seat->name, strerror(-r));

         }

-        if (error < 0)

-                log_debug("sysview: error while removing seat '%s': %s",

-                          seat->name, strerror(-error));

         sysview_seat_free(seat);

-        return error;

 }

 int sysview_context_new(sysview_context **out,

@@ -923,12 +904,12 @@ static int context_ud_hotplug(sysview_context *c, struct udev_device *d) {

                 if (!device)

                         return 0;

-                return context_remove_device(c, device);

+                context_remove_device(c, device);

         } else if (streq_ptr(action, "change")) {

                 if (!device)

                         return 0;

-                return context_change_device(c, device, d);

+                context_change_device(c, device, d);

         } else if (!action || streq_ptr(action, "add")) {

                 struct udev_device *p;

                 unsigned int type, t;

@@ -966,7 +947,7 @@ static int context_ud_hotplug(sysview_context *c, struct udev_device *d) {

                         return r;

                 }

-                return context_add_device(c, device);

+                context_add_device(c, device);

         }

         return 0;

@@ -1093,7 +1074,8 @@ static int context_ld_seat_new(sysview_context *c, sd_bus_message *signal) {

                 return r;

         }

-        return context_add_seat(c, id);

+        context_add_seat(c, id);

+        return 0;

 }

 static int context_ld_seat_removed(sysview_context *c, sd_bus_message *signal) {

@@ -1112,7 +1094,8 @@ static int context_ld_seat_removed(sysview_context *c, sd_bus_message *signal) {

         if (!seat)

                 return 0;

-        return context_remove_seat(c, seat);

+        context_remove_seat(c, seat);

+        return 0;

 }

 static int context_ld_session_new(sysview_context *c, sd_bus_message *signal) {

@@ -1159,14 +1142,13 @@ static int context_ld_session_new(sysview_context *c, sd_bus_message *signal) {

         }

         r = context_raise_session_filter(c, id, seatid, username, uid);

-        if (r <= 0) {

-                if (r < 0)

-                        log_debug("sysview: cannot filter new session '%s' on seat '%s': %s",

-                                  id, seatid, strerror(-r));

-                return r;

-        }

+        if (r < 0)

+                log_debug("sysview: callback failed while filtering session '%s': %s",

+                          id, strerror(-r));

+        else if (r > 0)

+                context_add_session(c, seat, id);

-        return context_add_session(c, seat, id);

+        return 0;

 error:

         log_debug("sysview: failed retrieving information for new session '%s': %s",

@@ -1190,7 +1172,8 @@ static int context_ld_session_removed(sysview_context *c, sd_bus_message *signal

         if (!session)

                 return 0;

-        return context_remove_session(c, session);

+        context_remove_session(c, session);

+        return 0;

 }

 static int context_ld_manager_signal_fn(sd_bus *bus,

@@ -1265,9 +1248,7 @@ static int context_ld_list_seats_fn(sd_bus *bus,

                 if (r < 0)

                         goto error;

-                r = context_add_seat(c, id);

-                if (r != 0)

-                        return r;

+                context_add_seat(c, id);

                 r = sd_bus_message_exit_container(reply);

                 if (r < 0)

@@ -1328,15 +1309,11 @@ static int context_ld_list_sessions_fn(sd_bus *bus,

                 seat = sysview_find_seat(c, seatid);

                 if (seat) {

                         r = context_raise_session_filter(c, id, seatid, username, uid);

-                        if (r < 0) {

-                                log_debug("sysview: cannot filter listed session '%s' on seat '%s': %s",

-                                          id, seatid, strerror(-r));

-                                return r;

-                        } else if (r > 0) {

-                                r = context_add_session(c, seat, id);

-                                if (r != 0)

-                                        return r;

-                        }

+                        if (r < 0)

+                                log_debug("sysview: callback failed while filtering session '%s': %s",

+                                          id, strerror(-r));

+                        else if (r > 0)

+                                context_add_session(c, seat, id);

                 }

                 r = sd_bus_message_exit_container(reply);