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.
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;
return -ENOMEM;
seat = hashmap_get(m->seats, e);
+ if (!seat)
+ return 0;
}
- if (!seat)
- return 0;
-
*found = seat;
return 1;
}
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;
return -ENOMEM;
session = hashmap_get(m->sessions, e);
+ if (!session)
+ return 0;
}
- if (!session)
- return 0;
-
*found = session;
return 1;
}
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;
~ianmdlvl / elogind.git / commitdiff