[systemd-devel] [PATCH 2/2] fsckd daemon for inter-fsckd communication

[Zbigniew Jędrzejewski-Szmek]

On Wed, Feb 04, 2015 at 05:50:29PM +0100, Didier Roche wrote:
> Le 04/02/2015 17:24, Zbigniew Jędrzejewski-Szmek a écrit :
> 
> Thanks again for the quick review! Fixed if not commented (sorry,
> some issues were back due to the refactoring).
> >On Wed, Feb 04, 2015 at 05:06:45PM +0100, Didier Roche wrote:
> >>+typedef struct Clients {
> >>+        struct Manager *manager;
> >>+        int fd;
> >>+        dev_t devnum;
> >>+        size_t cur;
> >>+        size_t max;
> >>+        int pass;
> >>+        double percent;
> >>+        size_t buflen;
> >>+
> >>+        LIST_FIELDS(struct Clients, clients);
> >>+} Clients;
> >Would be better to call this Client.
> >
> >>+typedef struct Manager {
> >>+        sd_event *event;
> >>+        Clients *clients;
> >But still 'Client *clients' here.
> 
> Right, I can't decide (due to the list) what's the best one, what do
> you think?
We use singular in other cases like this.

> >>+                if (!client)
> >>+                        return log_oom();
> >>+                client->fd = new_client_fd;
> >>+                client->manager = m;
> >>+                LIST_PREPEND(clients, m->clients, client);
> >>+                r = sd_event_add_io(m->event, NULL, client->fd, EPOLLIN, progress_handler, client);
> >>+                if (r < 0) {
> >>+                        remove_client(&(m->clients), client);
> >>+                        return r;
> >>+                }
> >I think you can do this in opposite order, and then the clean-up will
> >not be necessary.
> I need to delete the client item and close the new fd (which is part
> of remove_client()), and I can't assign the add_io event before the
> client instantiated. So, it will be another free() + safe_close()
> rather then remove_client(). Does it makes sense still to add this?
I guess no.

Zbyszek