[systemd-devel] [PATCH 2/5] fsckd: Fix some error return values
From 689c3c1808bd286ed96d36e4fc7ff875e2477697 Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Tue, 10 Mar 2015 09:01:11 +0100
Subject: [PATCH 2/5] fsckd: Fix some error return values
Change slightly recent changes introduced by
0b02c7c36dbb6f2ec7434eb8d18e0410ee1cc74c and
d42688ef21a695f8a30b0d899dafdc6ff1ee0973:
- do not return 0 when log_oom() couldn't create the manager.
- returns 0 if we connect successfully to plymouth.
---
src/fsckd/fsckd.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
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;
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();
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;
}
LIST_PREPEND(clients, m-clients, c);
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/5] fsckd: Fix some error return values
Le 10/03/2015 11:41, Lennart Poettering a écrit :
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.
Thanks for the explanations, I dropped that patch. Note that I needed to
do a slight change to the paradigm in the replacement of patch 04_*
Didier
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/5] fsckd: Fix some error return values
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
