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 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel