On Wed, Nov 6, 2013 at 2:25 AM, Lennart Poettering <lenn...@poettering.net> wrote: > On Wed, 06.11.13 01:33, Tom Gundersen (t...@jklm.no) wrote: >> Short-term TODO: >> - make rtnl calls asynchronous > > Don't wait for too long for this! The longer you wait the more code you > have to rework for this. And you shouldn't underestimate how complex > changes from sync to async are...
Yeah, it's top of the list ;-) >> Gateway=192.168.1.1 >> Address=label@192.168.1.23/24 >> Address=fe80::9aee:94ff:fe3f:c618/64 > > Hmm, what's the plan regarding confguration of scopes and other > attributes of addresses? Is the "label@" syntax your invention or has > this been used elsewhere (I am not opposed to the syntax, just curious). Good question. The @ syntax is my invention, but i'm very happy to change it if anyone has a better suggestion. For the other properties we might want, I would really like to find a syntax to get them all on one line. I'll try figure out a more or less exhaustive list of the properties we might want to support and suggest a syntax for it. In the meantime I'll commit this without the "label@" support, as the rest should be uncontroversial, and then we can add back the labeling when we are sure it is the way we want it. >> + sd_event_add_io(m->event, >> + udev_monitor_get_fd(m->udev_monitor), >> + EPOLLIN, >> + manager_dispatch_link_udev, >> + (void *)m, >> + &m->udev_event_source); > > Missing return value check! (and cast to void* is unnecessary, in C > all pointers automatically downgrade to void* without casting anyway) > >> + if (r < 0) { >> + log_warning("Colud not parse config file %s: %s", >> filename, strerror(-r)); > > ^^^^^^ Typo > >> + return r; >> + } else >> + log_debug("Parsed configuration file %s", filename); >> + >> + network->filename = strdup(filename); > > OOM check missing! > >> +static void networks_free(Manager *manager) { >> + Network *network, *network_next; >> + >> + if (!manager) >> + return; >> + >> + LIST_FOREACH_SAFE(networks, network, network_next, >> manager->networks) { >> + network_free(network); >> + } > > I usually just use: > > while ((network = manager->networks)) > network_free(free); > > Which should do the job too. > >> +struct Link { >> + uint64_t ifindex; > > Hmm is this really an uint64_t? if_nametoindex(3) suggestes it's an > "unsigned"? > > But yeah, looks great. Commit. Thanks, will fix up your comments and commit. Cheers, Tom _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel