Le 28/01/2015 15:37, Zbigniew Jędrzejewski-Szmek a écrit :
On Wed, Jan 28, 2015 at 02:20:40PM +0100, Didier Roche wrote:
Hey Zbigniew,

Thanks for the quick feedbacks! Integrated your changes in the incoming patches. Just answered to some comments here:

 From d0f49b4e0994b9a1dd2738da18c5a2a6708b444f Mon Sep 17 00:00:00 2001
From: Didier Roche <didro...@ubuntu.com>
Date: Mon, 26 Jan 2015 15:35:50 +0100
Subject: [PATCH 01/12] fsckd daemon for inter-fsckd communication


+        fsckd_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0);
+        if (fsckd_fd < 0) {
+                log_warning_errno(errno, "Cannot open fsckd socket, we won't report 
fsck progress: %m");
+                return -errno;
+        }
+        if (connect(fsckd_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + 
strlen(sa.un.sun_path)) < 0) {
+                log_warning_errno(errno, "Cannot connect to fsckd socket, we won't 
report fsck progress: %m");
+                return -errno;
+        }
f = fdopen(fd, "r");
          if (!f) {
-                safe_close(fd);
+                log_warning_errno(errno, "Cannot connect to fsck, we won't report 
fsck progress: %m");
                  return -errno;
          }
There's a double close now, since both f and fsckd_fd refer to the same fd.

Actually no, fd is the fd from the pipe between fsck -> systemd-fsck
and fsckd_fd is the fd from the socket between systemd-fsck -> systemd-fsckd.

Or am I missing something?


\ No newline at end of file
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
new file mode 100644
index 0000000..3059c68
--- /dev/null
+++ b/src/fsckd/fsckd.c
@@ -0,0 +1,295 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+}
+
+static int handle_requests(int socket_fd) {
+        Clients *first = NULL;
+        usec_t last_activity = 0;
+        int numdevices = 0, clear = 0;
+        double percent = 100;
+        _cleanup_fclose_ FILE *console = NULL;
NULL not necessary (and you don't have it in similar circumstances above ;))
So, after the discussion on the mailing list and to be more future proof, I went to recheck, but I'm afraid that I didn't find any place where I have some _cleanup_* without initializing to NULL? Did I get it wrong?


+                                log_error_errno(errno, "Socket read error, 
disconnecting fd %d: %m", current->fd);
+                                current->fd = 0;
0 is a valid file descriptor number. You must use -1. This also means that some 
initialization
to -1 is missing.


The clients are always initialized when we get a valid fd (current->fd = new_client_fd;), so no need to initiliaze to -1, I'm using now -1 though to invalidate the fd.

+static int create_socket(void) {
Can you base this on make_socket_fd() instead?

Oh nice, didn't find it when I looked for a generic utility. Using it :)


Thanks again for this review!
Cheers,
Didier
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to