On Thu, Jan 29, 2015 at 06:37:53PM +0100, Didier Roche wrote:
> 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.
You're right. Seems to be fine.
> 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?
fsckd_fd variable.
> >
> >>+ 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.
OK.
> >>+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 :)
Zbyszek
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
