On Tue, 09.12.14 00:03, WaLyong Cho (walyong....@samsung.com) wrote:
> } else {
> log_error("Unknown assignment %s.", assignment);
> return -EINVAL;
> diff --git a/src/run/run.c b/src/run/run.c
> index 85eb052..e145784 100644
> --- a/src/run/run.c
> +++ b/src/run/run.c
> @@ -30,6 +30,7 @@
> #include "env-util.h"
> #include "path-util.h"
> #include "bus-error.h"
> +#include "calendarspec.h"
>
> static bool arg_scope = false;
> static bool arg_remain_after_exit = false;
> @@ -47,27 +48,45 @@ static int arg_nice = 0;
> static bool arg_nice_set = false;
> static char **arg_environment = NULL;
> static char **arg_property = NULL;
> +static bool with_timer = false;
Hmm, maybe we should turn this boolean into a function? I mean, static
variables are something we should keep at a minimum. We do them for
command line arguments, since they are kinda global anyway, but I
think we should otherwise avoid them, especially if they are
effectively redundant anyway.
Hence, my suggestions would be to turn this into a call:
static bool with_timer(void) {
return arg_on_active || arg_on_boot || arg_on_startup ||
arg_on_unit_active || arg_on_unit_inactive || arg_on_calendar;
}
Or so?
> +static int start_transient_service(
> + sd_bus *bus,
> + char **argv,
> + sd_bus_error *error) {
> +
> + _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
> + _cleanup_free_ char *service = NULL;
> + char *state = NULL;
> + int r;
> +
> + assert(bus);
> + assert(argv);
> +
> + if (arg_unit) {
> + service = unit_name_mangle_with_suffix(arg_unit,
> MANGLE_NOGLOB, ".service");
> + if (!service)
> + return log_oom();
> +
> + if (get_unit_state_by_name(bus, service, &state) == 0) {
> + log_error("Unit %s is already exist with %s state.",
> service, state);
> + return -EEXIST;
> + }
What's the rationale for checking the unit state here? I mean, PID 1
should fail anyway in this case, do we realy need to check this name
in advance? I mean, such a check is necessarily racy, since the unit
might appear while between our client-side check and the actual
execution of the service, no?
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
