[PATCH] act: grab a reference when adding an action to a filter

Cong Wang xiyou.wangcong at gmail.com
Tue Apr 15 15:52:31 PDT 2014


On Tue, Apr 15, 2014 at 4:13 AM, Thomas Haller <thaller at redhat.com> wrote:
>
> How about adding a function rtnl_act_get() -- analog to the existing
> rtnl_act_put() function?
>

Makes sense for me.

>
>
> Again this is a behavioral change.
>
> I think the behavior should be analog to other functions in libnl. Also
> note, that now rtnl_u32_del_action() removes the object and ownership
> goes back to the caller. I think this is inconsistent.
> If course rtnl_u32_del_action() releasing the reference might mean, that
> the action is gone when the function returns (but I would kind of expect
> that).

Yes, you are right.

>
> Even if the behavior of the function is inconsistent, breaking API is so
> annoying, that we might not fix this? What do you think? We could
> justify the API break it by saying that pre-libnl3.2.25 these functions
> were new and simply buggy.
>
> In any case, it should be explicitly mentioned in the documentation and
> you could add NL_CAPABILITY_RTNL_U32_ADD_ACTION_OWN_REFERENCE (or
> similarly named) to indicate this API change.
>


I think the best choice is adding such a capability flag just in case...

>
> The same applies to basic.c.
>
>
>
> Unrelated to this, looking at u32_clone(), it does not create a deep
> copy of cu_act (which it maybe should??) nor does it get additional
> references to the objects. It should do either one.
>


We need to copy the whole linked list, I plan to fix it later and separately.

Thanks!



More information about the libnl mailing list