search

thom311 / libnl

Netlink Library Suite
GNU Lesser General Public License v2.1
419 stars 313 forks source link

route: fix bug in 'rntl_act_next()' #318

Closed voldymyr closed 2 years ago

voldymyr commented 2 years ago

Make sure the 'next' action is non-null and 'get' it with rtnl_act_get().

Having completed the work with action the user is responsible to call rtnl_act_put()

Signed-off-by: Volodymyr Bendiuga volodymyr.bendiuga@westermo.com

thom311 commented 2 years ago

Whether a function returns a reference (ownership) or not, is up to the function. In libnl, that is unfortunately not done consistently, and usually it's also unnecessary to return a reference. There is really no need that a function rtnl_act_next() should return a reference (except maybe an argument for consistency with other API).

In any case, changing whether a function returns ownership or not, is an API change (because it changes whether the caller is supposed to return the reference).

Why do you think this function should return a reference? This function exists now for 5 years, we cannot break API this way.

Also, inside libnl source tree, there is only one caller of this function (mall_clone()). And this caller does not expect that it gets ownership of the reference, so this change will introduce a bug there.

thom311 commented 2 years ago

As said, I think this patch is not something we can change, because it breaks existing API. Also, it causes a leak in mall_clone().

I will close this MR. If you have anything to add, please comment or reopen.

Thank you!!