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

[Didier Roche]

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?
>> +                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?

>
>> +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