2014/1/5 Zbigniew Jędrzejewski-Szmek <[email protected]>: > Looks great, except for one issue: > > On Sat, Jan 04, 2014 at 02:35:27AM +0100, Simon Peeters wrote: >> @@ -1865,14 +1863,11 @@ finish: >> watchdog_close(false); >> >> /* Tell the binary how often to ping */ >> - snprintf(e, sizeof(e), "WATCHDOG_USEC=%llu", >> (unsigned long long) arg_shutdown_watchdog); >> - char_array_0(e); >> + asprintf(&e, "WATCHDOG_USEC=%llu", (unsigned long >> long) arg_shutdown_watchdog); >> >> - env_block = strv_append(environ, e); >> - } else { >> - env_block = strv_copy(environ); >> + strv_push(&env_block, e); > Should there be oom handling here?
there wasn't any in place, since this is shutdown code, I think we just need to avoid segfaulting on oom. but indeed, asprintf() doesn't set e to null on oom, so that should be: if (asprintf(&e, "WATCHDOG_USEC="USEC_FMT, arg_shutdown_watchdog) < 0) e = NULL; and then it is equivalently oom safe as before, which means in worst case passing a NULL env to the shutdown binary. _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
