Hi Marcel, >> +LIB_EXPORT bool l_genl_msg_builder_enter_nested( >> + struct l_genl_msg_builder *builder, >> + uint16_t type) >> +{ >> + struct nlattr *nla; >> + >> + if (unlikely(!builder)) >> + return false; >> + >> + if (!builder->nests) { >> + builder->nests = l_queue_new(); >> + if (!builder->nests) >> + return false; >> + } > do we really need a queue here? Can we not just store the latest offset. Also how do you do nested of nested attributes? Nested of nested exists, in nftables for instance. Unless we prefer to have a fixed depth of nested of nested (like 3, 4...?), we need such dynamically allocated list. I haven't tested this case, but we have to handle it. Unless we plan to use that code for nl80211 only. >> + } >> + >> + // In case nothing has been appended into the nested attribute >> + // it could try to be smart and revert the message back and return true > this is not our comment style. Please respect the coding style. Actually, it's more like a question that needs an answer. I don't want this comment to go in anyway. Or let's just forget entirely about this. >> +struct l_genl_msg *l_genl_msg_builder_finalize( >> + struct l_genl_msg_builder *builder); > So I saw that l_dbus_message_builder does a similar thing. We give in a message to start from and then we get that message back out when finalizing it. Has anything changed in between or why are we doing this? The rule was to follow what DBus API does. But I think it's useless: - l_genl_msg_builder_new() it could take a size, and create the msg internally. (or have a fixed size of 512bytes, and if really needed would realloc relevantly?) - then if everything went well: l_genl_msg_builder_finalize() would return a msg pointer (a reference of it actually), which one we could use to send (and send unref the msg). Less function to call from client code. It's actually awkward that you have to create the msg yourself, then also a builder with it etc... when you have also other functions to append attributes directly in the msg. At least in dbus api you can't append anything BUT through a builder. (So yes I don't want to follow the DBus Api but finally a bit anyway...) The naming is too long as well, l_genl_msg_builder_append_attribute() mostly. Suggestions would be welcome. I can strip the "ibute" at the end at least. And also msg_builder into _msgbld_ or does the lack of vowels makes it even crappier? Tomasz