On Sun, Jan 05, 2014 at 12:52:31PM +0100, Peeters Simon wrote:
> 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;
Right. I added the inverse of this (if (asprintf() > 0) strv_push()).

Applied.

Zbyszek

_______________________________________________
systemd-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to