[systemd-devel] [PATCH 2/3] timer: timer can be a transient unit

[Lennart Poettering]

On Tue, 07.10.14 14:20, WaLyong Cho (walyong.cho at samsung.com) wrote:

> +        assert(message);
> +
> +        if (streq(name, "OnActiveSec") ||
> +            streq(name, "OnBootSec") ||
> +            streq(name, "OnStartupSec") ||
> +            streq(name, "OnUnitActiveSec") ||
> +            streq(name, "OnUnitInactiveSec")) {

I think it would be cool to use STR_IN_SET() here.

> +
> +                TimerValue *v;
> +                TimerBase b = _TIMER_BASE_INVALID;
> +                usec_t u = 0;
> +                CalendarSpec *c = NULL;
> +
> +                b = timer_base_from_string(name);
> +                if (b < 0)
> +                        return 0;
> +
> +                r = sd_bus_message_read(message, "s", &str);
> +                if (r < 0)
> +                        return r;
> +
> +                if (mode != UNIT_CHECK) {
> +                        if (b == TIMER_CALENDAR) {

Hmm, there's something wrong here, b can never be TIMER_CALENDAR as
we never enter this if block for name == "OnCalendar".

> +
> +        } else if (streq(name, "AccuracySec")) {
> +
> +                usec_t u = 0;
> +
> +                r = sd_bus_message_read(message, "s", &str);
> +                if (r < 0)
> +                        return r;

No. The accuracy should be of type "t", and be called
"AccuracyUSec". There's a slight asymmetry between dbus interfaces and
the config files here, as the bus interfaces always expose usec as
uint64_t, while the config files default to sec as unit...

>  
> +        /* If trigger unit is transient, make sure be gabage
> +         * collected. That maybe have no_gc to wait until this timer
> +         * is started. */
> +        if (UNIT_TRIGGER(u)->transient)
> +                UNIT_TRIGGER(u)->no_gc = false;
> +

No, this should be unnecessary?

timer units should just follow the normal GC logic, and get GC'ed as
soon as they are not running or referenced anymore.

Lennart

-- 
Lennart Poettering, Red Hat