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.
> + if ((fabs(current_percent - m->percent) > 0.01) ||
> (current_numdevices != m->numdevices)) {
Again, too much parens.
> + client = malloc(sizeof(Clients));
new0(Client, 1)
(Unless you initialize all fields, it's better to zero out the structure.)
> + 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.
> + m = new0(Manager, 1);
> + if (!m)
> + return -ENOMEM;
> +
> + r = sd_event_default(&m->event);
> + if (r < 0)
> + return r;
> +
> + m->clients = NULL;
> + m->connection_fd = fd;
> +
> + m->console = fopen("/dev/console", "we");
> + if (!m->console)
> + return log_error_errno(errno, "Can't connect to
> /dev/console: %m");
> + m->percent = 100;
> + m->numdevices = 0;
> + m->clear = 0;
Since structure is initialized to zero, there's no need to set stuff to 0 again.
> +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.
Zbyszek
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
