Hi Tim, >> >> So to me this pid argument seems unnecessary. Also, this change is side- >> effecting l_genl_new(int fd) method which you are removing. While nobody >> is using it at the moment, removing it in this fashion is poor style (since >> l_genl_set_close_on_unref is mostly meant for this constructor) > > FYI: This was your suggestion to my original patch . Was it? Don't recall. I was probably under the assumption that pid setting was required. I do reserve the right to change my mind, especially when looking at things more closely ;) I'm picking on the procedural aspect. E.g. if we remove something, then we should make sure it is done thoroughly (maybe even in a separate commit) and hopefully mention it in the commit description. Once you start using git blame and hunting for why a piece of code looks the way it looks, having nice git history becomes paramount. >>> >>> +bool l_genl_set_unicast_handler(struct l_genl *genl, >>> + l_genl_msg_func_t handler, >>> + void *user_data, >>> + l_genl_destroy_func_t >> destroy); >>> + >>> +bool l_genl_unset_unicast_handler(struct l_genl *genl); >> >> I rather we simply use l_genl_set_unicast_handler with NULL handler >> argument to accomplish this. > > I was thinking about it, but then the API does not look self-explanatory. > This is inconsistent with the rest of ell API though. For example, see l_io_set_read_handler, l_io_set_write_handler, l_io_set_disconnect_handler, etc. Any clarifying details can also be added to the docs. Regards, -Denis