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?
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?+ 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.
+typedef struct FsckProgress { + dev_t devnum; + unsigned long cur; + unsigned long max; + int pass; +} FsckProgress;I think I asked about that before, but not 100% sure: shouldn't those be size_t? If they are disk offsets, than long is not enough.
Sorry, I only changed it in fsckd.c and fsck.c, done in the passing struct now.
Cheers, Didier _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
