[...]
>>
>> I would say:
>>
>> * Returns 0 on success, -1 on error. Additionally, if the @error is
>> * non-zero, then the failure occurred during virNetlinkCommand, but
>> * no error message generated leaving it up to the caller to handle
>> * the condition.
>
> "is generated" I guess?
>
> Anyway, I agree.
>
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
