From 04bd404dffc8bc1ec5c3fed91e75fad1a1b1a4d3 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 1 May 2019 13:15:57 +0200
Subject: [PATCH] platform: merge _add_action(), _add_action_simple() and
 _add_action_mirred() into _nl_msg_new_tfilter()

There is only one caller, hence it's simpler to see it all in one place.
I prefer this, because then I can read the code top to bottom and
see what's happening, without following helper functions.

Also, this way we can "reuse" the nla_put_failure label and assertion. Previously,
if the assertion was hit we would not rewind the buffer but continue
constructing the message (which is already borked). Not that it matters
too much, because this was on an "failed-assertion" code path.
---
 src/platform/nm-linux-platform.c | 125 ++++++++++++-------------------
 1 file changed, 46 insertions(+), 79 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index b0000e0762..6c1dbd0bed 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -4260,83 +4260,6 @@ nla_put_failure:
 	g_return_val_if_reached (NULL);
 }
 
-static gboolean
-_add_action_simple (struct nl_msg *msg,
-                    const NMPlatformActionSimple *simple)
-{
-	struct nlattr *act_options;
-	struct tc_defact sel = { 0, };
-
-	if (!(act_options = nla_nest_start (msg, TCA_ACT_OPTIONS)))
-		goto nla_put_failure;
-
-	NLA_PUT (msg, TCA_DEF_PARMS, sizeof (sel), &sel);
-	NLA_PUT (msg, TCA_DEF_DATA, sizeof (simple->sdata), simple->sdata);
-
-	nla_nest_end (msg, act_options);
-
-	return TRUE;
-
-nla_put_failure:
-	g_return_val_if_reached (FALSE);
-}
-
-static gboolean
-_add_action_mirred (struct nl_msg *msg,
-                    const NMPlatformActionMirred *mirred)
-{
-	struct nlattr *act_options;
-	struct tc_mirred sel = { 0, };
-
-	if (!(act_options = nla_nest_start (msg, TCA_ACT_OPTIONS)))
-		goto nla_put_failure;
-
-	if (mirred->egress && mirred->redirect)
-		sel.eaction = TCA_EGRESS_REDIR;
-	else if (mirred->egress && mirred->mirror)
-		sel.eaction = TCA_EGRESS_MIRROR;
-	else if (mirred->ingress && mirred->redirect)
-		sel.eaction = TCA_INGRESS_REDIR;
-	else if (mirred->ingress && mirred->mirror)
-		sel.eaction = TCA_INGRESS_MIRROR;
-	sel.ifindex = mirred->ifindex;
-
-	NLA_PUT (msg, TCA_MIRRED_PARMS, sizeof (sel), &sel);
-
-	nla_nest_end (msg, act_options);
-
-	return TRUE;
-
-nla_put_failure:
-	g_return_val_if_reached (FALSE);
-}
-
-static gboolean
-_add_action (struct nl_msg *msg,
-             const NMPlatformAction *action)
-{
-	struct nlattr *prio;
-
-	nm_assert (action || action->kind);
-
-	if (!(prio = nla_nest_start (msg, 1 /* priority */)))
-		goto nla_put_failure;
-
-	NLA_PUT_STRING (msg, TCA_ACT_KIND, action->kind);
-
-	if (nm_streq (action->kind, NM_PLATFORM_ACTION_KIND_SIMPLE))
-		_add_action_simple (msg, &action->simple);
-	else if (nm_streq (action->kind, NM_PLATFORM_ACTION_KIND_MIRRED))
-		_add_action_mirred (msg, &action->mirred);
-
-	nla_nest_end (msg, prio);
-
-	return TRUE;
-
-nla_put_failure:
-	g_return_val_if_reached (FALSE);
-}
-
 static struct nl_msg *
 _nl_msg_new_tfilter (int nlmsg_type,
                      int nlmsg_flags,
@@ -4366,8 +4289,52 @@ _nl_msg_new_tfilter (int nlmsg_type,
 	if (!(act_tab = nla_nest_start (msg, TCA_OPTIONS))) // 3 TCA_ACT_KIND TCA_ACT_KIND
 		goto nla_put_failure;
 
-	if (tfilter->action.kind)
-		_add_action (msg, &tfilter->action);
+	if (tfilter->action.kind) {
+		const NMPlatformAction *action = &tfilter->action;
+		struct nlattr *prio;
+		struct nlattr *act_options;
+
+		if (!(prio = nla_nest_start (msg, 1 /* priority */)))
+			goto nla_put_failure;
+
+		NLA_PUT_STRING (msg, TCA_ACT_KIND, action->kind);
+
+		if (nm_streq (action->kind, NM_PLATFORM_ACTION_KIND_SIMPLE)) {
+			const NMPlatformActionSimple *simple = &action->simple;
+			struct tc_defact sel = { 0, };
+
+			if (!(act_options = nla_nest_start (msg, TCA_ACT_OPTIONS)))
+				goto nla_put_failure;
+
+			NLA_PUT (msg, TCA_DEF_PARMS, sizeof (sel), &sel);
+			NLA_PUT (msg, TCA_DEF_DATA, sizeof (simple->sdata), simple->sdata);
+
+			nla_nest_end (msg, act_options);
+
+		} else if (nm_streq (action->kind, NM_PLATFORM_ACTION_KIND_MIRRED)) {
+			const NMPlatformActionMirred *mirred = &action->mirred;
+			struct tc_mirred sel = { 0, };
+
+			if (!(act_options = nla_nest_start (msg, TCA_ACT_OPTIONS)))
+				goto nla_put_failure;
+
+			if (mirred->egress && mirred->redirect)
+				sel.eaction = TCA_EGRESS_REDIR;
+			else if (mirred->egress && mirred->mirror)
+				sel.eaction = TCA_EGRESS_MIRROR;
+			else if (mirred->ingress && mirred->redirect)
+				sel.eaction = TCA_INGRESS_REDIR;
+			else if (mirred->ingress && mirred->mirror)
+				sel.eaction = TCA_INGRESS_MIRROR;
+			sel.ifindex = mirred->ifindex;
+
+			NLA_PUT (msg, TCA_MIRRED_PARMS, sizeof (sel), &sel);
+
+			nla_nest_end (msg, act_options);
+		}
+
+		nla_nest_end (msg, prio);
+	}
 
 	nla_nest_end (msg, tc_options);