route/cls: add get/take wrappers for rtnl_act_append()

rtnl_act_append() either takes ownership of the argument, or does
nothing (on error). This pattern is hard to get right. In the past,
there were various bugs at this place.

Add two wrappers _rtnl_act_append_get() and _rtnl_act_append_take()
which consistently don't take ownership of the pointer or take it.

Also, in functions like rtnl_flower_append_action() only set the mask
after successfully modifying the data.

include/nl-aux-route/nl-route.h

@@ -5,6 +5,8 @@
 
 #include "base/nl-base-utils.h"
 
+#include <netlink/route/action.h>
+
 struct rtnl_link;
 void rtnl_link_put(struct rtnl_link *);
 #define _nl_auto_rtnl_link _nl_auto(_nl_auto_rtnl_link_fcn)
@@ -40,13 +42,10 @@ void rtnl_link_af_ops_put(struct rtnl_link_af_ops *);
 _NL_AUTO_DEFINE_FCN_TYPED0(struct rtnl_link_af_ops *,
 			   _nl_auto_rtnl_link_af_ops_fcn, rtnl_link_af_ops_put);
 
-struct rtnl_act;
-void rtnl_act_put(struct rtnl_act *);
 #define _nl_auto_rtnl_act _nl_auto(_nl_auto_rtnl_act_fcn)
 _NL_AUTO_DEFINE_FCN_TYPED0(struct rtnl_act *, _nl_auto_rtnl_act_fcn,
 			   rtnl_act_put);
 
-void rtnl_act_put_all(struct rtnl_act **head);
 #define _nl_auto_rtnl_act_all _nl_auto(_nl_auto_rtnl_act_fcn_all)
 _NL_AUTO_DEFINE_FCN_INDIRECT0(struct rtnl_act *, _nl_auto_rtnl_act_fcn_all,
 			      rtnl_act_put_all);
@@ -64,4 +63,28 @@ void rtnl_cls_put(struct rtnl_cls *);
 _NL_AUTO_DEFINE_FCN_TYPED0(struct rtnl_cls *, _nl_auto_rtnl_cls_fcn,
 			   rtnl_cls_put);
 
+/*****************************************************************************/
+
+static inline int _rtnl_act_append_get(struct rtnl_act **head,
+				       struct rtnl_act *new)
+{
+	int r;
+
+	r = rtnl_act_append(head, new);
+	if (r >= 0)
+		rtnl_act_get(new);
+	return r;
+}
+
+static inline int _rtnl_act_append_take(struct rtnl_act **head,
+					struct rtnl_act *new)
+{
+	int r;
+
+	r = rtnl_act_append(head, new);
+	if (r < 0)
+		rtnl_act_put(new);
+	return r;
+}
+
 #endif /* __NETLINK_NL_AUX_ROUTE_NL_ROUTE_H__ */

lib/route/act.c

@@ -493,11 +493,9 @@ int rtnl_act_parse(struct rtnl_act **head, struct nlattr *tb)
 			if (err < 0)
 				return err;
 		}
-		err = rtnl_act_append(&tmp_head, act);
+		err = _rtnl_act_append_take(&tmp_head, _nl_steal_pointer(&act));
 		if (err < 0)
 			return err;
-
-		_nl_steal_pointer(&act);
 	}
 
 	*head = _nl_steal_pointer(&tmp_head);

lib/route/cls/basic.c

@@ -25,6 +25,7 @@
 #include <netlink/route/cls/ematch.h>
 
 #include "tc-api.h"
+#include "nl-aux-route/nl-route.h"
 
 struct rtnl_basic
 {
@@ -223,12 +224,10 @@ int rtnl_basic_add_action(struct rtnl_cls *cls, struct rtnl_act *act)
 	if (!(b = rtnl_tc_data(TC_CAST(cls))))
 		return -NLE_NOMEM;
 
-	b->b_mask |= BASIC_ATTR_ACTION;
-	if ((err = rtnl_act_append(&b->b_act, act)))
+	if ((err = _rtnl_act_append_get(&b->b_act, act)) < 0)
 		return err;
 
-	/* In case user frees it */
-	rtnl_act_get(act);
+	b->b_mask |= BASIC_ATTR_ACTION;
 	return 0;
 }
 

lib/route/cls/flower.c

@@ -15,6 +15,7 @@
 #include <netlink/route/cls/flower.h>
 
 #include "tc-api.h"
+#include "nl-aux-route/nl-route.h"
 
 /** @cond SKIP */
 struct rtnl_flower {
@@ -817,12 +818,10 @@ int rtnl_flower_append_action(struct rtnl_cls *cls, struct rtnl_act *act)
 	if (!(f = rtnl_tc_data(TC_CAST(cls))))
 		return -NLE_NOMEM;
 
-	f->cf_mask |= FLOWER_ATTR_ACTION;
-
-	if ((err = rtnl_act_append(&f->cf_act, act)) < 0)
+	if ((err = _rtnl_act_append_get(&f->cf_act, act)) < 0)
 		return err;
 
-	rtnl_act_get(act);
+	f->cf_mask |= FLOWER_ATTR_ACTION;
 	return 0;
 }
 

lib/route/cls/mall.c

@@ -20,6 +20,7 @@
 #include <netlink/route/action.h>
 
 #include "tc-api.h"
+#include "nl-aux-route/nl-route.h"
 
 struct rtnl_mall {
 	uint32_t m_classid;
@@ -107,12 +108,10 @@ int rtnl_mall_append_action(struct rtnl_cls *cls, struct rtnl_act *act)
 	if (!(mall = rtnl_tc_data(TC_CAST(cls))))
 		return -NLE_NOMEM;
 
-	mall->m_mask |= MALL_ATTR_ACTION;
-	err = rtnl_act_append(&mall->m_act, act);
-	if (err < 0)
-	        return err;
+	if ((err = _rtnl_act_append_get(&mall->m_act, act)) < 0)
+		return err;
 
-	rtnl_act_get(act);
+	mall->m_mask |= MALL_ATTR_ACTION;
 	return 0;
 }
 
@@ -245,7 +244,7 @@ static int mall_clone(void *_dst, void *_src)
 			if (!new)
 				return -NLE_NOMEM;
 
-			err = rtnl_act_append(&dst->m_act, new);
+			err = _rtnl_act_append_take(&dst->m_act, new);
 			if (err < 0)
 				return err;
 

lib/route/cls/u32.c

@@ -612,12 +612,10 @@ int rtnl_u32_add_action(struct rtnl_cls *cls, struct rtnl_act *act)
 	if (!(u = rtnl_tc_data(TC_CAST(cls))))
 		return -NLE_NOMEM;
 
-	u->cu_mask |= U32_ATTR_ACTION;
-	if ((err = rtnl_act_append(&u->cu_act, act)))
+	if ((err = _rtnl_act_append_get(&u->cu_act, act)) < 0)
 		return err;
 
-	/* In case user frees it */
-	rtnl_act_get(act);
+	u->cu_mask |= U32_ATTR_ACTION;
 	return 0;
 }