Re: [libvirt] [PATCHv3 1/3] util: netlink: Introduce virNetlinkNewLink helper

[...] right... fingers don't always comply with mind ;-) [...] >>>> + if ((STREQ(type, "macvtap") || STREQ(type, "macvlan")) && >>>> + extra_args && >>>> + extra_args->macvlan_mode && >>>> + *extra_args->macvlan_mode > 0) { >> Why is @macvlan_mode a "const uint32_t *", doesn't need to be does it? >>>> + if (!(infodata = nla_nest_start(nl_msg, IFLA_INFO_DATA))) >>>> + goto buffer_too_small; >>>> + >>>> + if (nla_put_u32(nl_msg, IFLA_MACVLAN_MODE, *extra_args->macvlan_mode) < 0) >>>> here too... >>>>> + goto buffer_too_small; >>>> + >>>> + nla_nest_end(nl_msg, infodata); >>>> + } >>>> + >>>> + nla_nest_end(nl_msg, linkinfo); >>>> + >>>> + if (extra_args) { >>>> + if (extra_args->ifindex && >>>> + nla_put_u32(nl_msg, IFLA_LINK, *extra_args->ifindex) < 0) >>>> and here as well.... >>> Similarly @ifindex doesn't seem to need to be a "const int *" > > Are you referring to the const correctness or the fact it's a pointer? If it's > the former, then I don't see a problem, if it's the latter, then I simply > wanted a deterministic way of telling that an argument is set, in case values > 0, -1, etc. had some meaning. > latter that it's a pointer. The way it looks to me is that the value could be changed in the function, but I understand the point of 0 (not supplied) vs. NULL... So fair enough reason to keep as const int *... John