On Tue, 10.03.15 11:33, Didier Roche (didro...@ubuntu.com) wrote: > diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c > index 3fc923b..9393379 100644 > --- a/src/fsckd/fsckd.c > +++ b/src/fsckd/fsckd.c > @@ -220,7 +220,7 @@ static int manager_connect_plymouth(Manager *m) { > goto fail; > } > > - return 1; > + return 0;
Oh, well, this doesn't matter too much I guess, since we don't check for the precise value, but just < 0 or >= 0... Returning 1 in this case is actually a bit of a pattern I am trying to adopt across the codebase: return 0 if all is OK but we didn't do anything. Return > 1 if all is OK and we actually did something. In this case this distinction is of course entirely irrelevant, but it was more of an automatism... > > fail: > manager_disconnect_plymouth(m); > @@ -406,10 +406,8 @@ static int > manager_new_connection_handler(sd_event_source *s, int fd, uint32_t r > log_debug("New fsck client connected to fd: %d", new_client_fd); > > c = new0(Client, 1); > - if (!c) { > - log_oom(); > - return 0; > - } > + if (!c) > + return log_oom(); Well, I think logging and ignoring the OOM condition in this case is a good idea. THink about the effect of returning an error up here: when a handler callback returns an error sd-event will disable the event source automatically. This means if fsckd hits an OOM once here, it will become completely unresponsive, as it never processes incoming connections anymore. However, if we simply log, close the connection, and continue without propagating an error up, we are slightly more robust: sure, the connection will fail, but subsequent connections might not... > c->fd = new_client_fd; > new_client_fd = -1; > @@ -417,7 +415,7 @@ static int manager_new_connection_handler(sd_event_source > *s, int fd, uint32_t r > r = sd_event_add_io(m->event, &c->event_source, c->fd, EPOLLIN, > client_progress_handler, c); > if (r < 0) { > log_oom(); > - return 0; > + return r; Same here. Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel