[systemd-devel] [RFC] logind: introduce session "positions"

[Lennart Poettering]

On Sun, 01.12.13 12:43, David Herrmann (dh.herrmann at gmail.com) wrote:

> +static int method_switch_to(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
> +        Seat *s = userdata;
> +        unsigned int to;
> +        int r;
> +
> +        assert(bus);
> +        assert(message);
> +        assert(s);
> +
> +        r = sd_bus_message_read(message, "u", &to);
> +        if (r < 0)
> +                return r;
> +
> +        if (!to)
> +                return -EINVAL;

Nitpick: To check numeric values against 0, please use explicit comparison:

if (to <= 0)
        return -EINVAL;

or so... 

if (to) 

or 

if (!to)

is fine for bools and pointers, but for numbers I'd prefer explicit
numeric checks...

> +static void seat_assign_position(Seat *s, Session *session) {
> +        unsigned int pos;
> +
> +        if (session->pos > 0)
> +                return;
> +
> +        for (pos = 1; pos < s->position_count; ++pos)
> +                if (!s->positions[pos])
> +                        break;

I wonder if it wouldn't be preferable to always allocate an ID at the
end (i.e. not fill up holes in the middle)? This might result in more
holes, but suggests chronological ordering by default, which might be a
good thing to do?

Looks good though. Go ahead and commit.

Lennart

-- 
Lennart Poettering, Red Hat