summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorAlan Jenkins <alan.christopher.jenkins@gmail.com>2017-10-14 09:25:56 +0100
committerSven Eden <yamakuzure@gmx.net>2017-10-14 09:25:56 +0100
commit813ee46c0ec85d7d93687b3660a4f4885c43ceae (patch)
tree69e126a16edd2e34c734371c3597654591ec479f /src
parent0f48edebdb679406a22f1d4ce81be64f0bb32c5b (diff)
logind: "self" objects which do not apply - return specific error messages
It's confusing that the bus API has aliases like "session/self" that return an error based on ENXIO, when it also has methods that return e.g. NO_SESSION_FOR_PID for the same problem. The latter kind of error includes more specifically helpful messages. "user/self" is the odd one out; it returns a generic UnknownObject error when it is not applicable to the caller. It's not clear whether this was intentional, but at first I thought it was more correct. More specifically, user_object_find() was returning 0 for "user/self", in the same situations (more or less) where user_node_enumerator() was omitting "user/self". I thought that was a good idea, because returning e.g. -ENXIO instead suggested that there _is_ something specific on that path. And it could be confused with errors of the method being called. Therefore I suggested changing the enumerator, always admitting that there is a handler for the path "foo/self", but returning a specific error when queried. However this interacts poorly with tools like D-Feet or `busctl`. In either tool, looking at logind would show an error message, and then go on to omit "user/self" in the normal listing. These tools are very useful, so we don't want to interfere with them. I think we can change the error codes without causing problems. The self objects were not listed in the documentation. They have been suggested to other projects - but without reference to error reporting. "seat/self" is used by various Wayland compositors for VT switching, but they don't appear to reference specific errors. We _could_ insist on the link between enumeration and UnknownObject, and standardize on that as the error for the aliases. But I'm not aware of any practical complaints, that we returned an error from an object that didn't exist. Instead, let's unify the codepaths for "user/self" vs GetUserByPid(0) etc. We will return the most helpful error message we can think of, if the object does not exist. E.g. for "session/self", we might return an error that the caller does not belong to a session. If one of the compositors is ever simplified to use "session/self" in initialization, users would be able to trigger such errors (e.g. run `gnome-shell` inside gnome-terminal). The message text will most likely be logged. The user might not know what the "session" is, but at least we'll be pointing towards the right questions. I think it should also be clearer for development / debugging. Unifying the code paths is also slightly helpful for auditing / marking calls to sd_bus_creds_get_session() in subsequent commits.
Diffstat (limited to 'src')
-rw-r--r--src/login/logind-seat-dbus.c20
-rw-r--r--src/login/logind-session-dbus.c15
-rw-r--r--src/login/logind-user-dbus.c5
3 files changed, 7 insertions, 33 deletions
diff --git a/src/login/logind-seat-dbus.c b/src/login/logind-seat-dbus.c
index f934a5326..2154b878d 100644
--- a/src/login/logind-seat-dbus.c
+++ b/src/login/logind-seat-dbus.c
@@ -332,28 +332,15 @@ int seat_object_find(sd_bus *bus, const char *path, const char *interface, void
assert(m);
if (streq(path, "/org/freedesktop/login1/seat/self")) {
- _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
sd_bus_message *message;
- Session *session;
- const char *name;
message = sd_bus_get_current_message(bus);
if (!message)
return 0;
- r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_SESSION|SD_BUS_CREDS_AUGMENT, &creds);
- if (r < 0)
- return r;
-
- r = sd_bus_creds_get_session(creds, &name);
+ r = manager_get_seat_from_creds(m, message, NULL, error, &seat);
if (r < 0)
return r;
-
- session = hashmap_get(m->sessions, name);
- if (!session)
- return 0;
-
- seat = session->seat;
} else {
_cleanup_free_ char *e = NULL;
const char *p;
@@ -367,11 +354,10 @@ int seat_object_find(sd_bus *bus, const char *path, const char *interface, void
return -ENOMEM;
seat = hashmap_get(m->seats, e);
+ if (!seat)
+ return 0;
}
- if (!seat)
- return 0;
-
*found = seat;
return 1;
}
diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 7c45dbe9d..4e3162d8f 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -580,23 +580,15 @@ int session_object_find(sd_bus *bus, const char *path, const char *interface, vo
assert(m);
if (streq(path, "/org/freedesktop/login1/session/self")) {
- _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
sd_bus_message *message;
- const char *name;
message = sd_bus_get_current_message(bus);
if (!message)
return 0;
- r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_SESSION|SD_BUS_CREDS_AUGMENT, &creds);
- if (r < 0)
- return r;
-
- r = sd_bus_creds_get_session(creds, &name);
+ r = manager_get_session_from_creds(m, message, NULL, error, &session);
if (r < 0)
return r;
-
- session = hashmap_get(m->sessions, name);
} else {
_cleanup_free_ char *e = NULL;
const char *p;
@@ -610,11 +602,10 @@ int session_object_find(sd_bus *bus, const char *path, const char *interface, vo
return -ENOMEM;
session = hashmap_get(m->sessions, e);
+ if (!session)
+ return 0;
}
- if (!session)
- return 0;
-
*found = session;
return 1;
}
diff --git a/src/login/logind-user-dbus.c b/src/login/logind-user-dbus.c
index 987c63014..921f1e0f9 100644
--- a/src/login/logind-user-dbus.c
+++ b/src/login/logind-user-dbus.c
@@ -270,18 +270,15 @@ int user_object_find(sd_bus *bus, const char *path, const char *interface, void
assert(m);
if (streq(path, "/org/freedesktop/login1/user/self")) {
- _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
sd_bus_message *message;
message = sd_bus_get_current_message(bus);
if (!message)
return 0;
- r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_OWNER_UID|SD_BUS_CREDS_AUGMENT, &creds);
+ r = manager_get_user_from_creds(m, message, UID_INVALID, error, &user);
if (r < 0)
return r;
-
- r = sd_bus_creds_get_owner_uid(creds, &uid);
} else {
const char *p;