[PATCH iproute2-next v3 6/8] iplink: Perform most of request buffer setups and checks in iplink_parse()

[Serhey Popovych <serhe.popovych@gmail.com>]

To benefit other users (e.g. link_veth.c) of iplink_parse() from
additional attribute checks and setups made in iplink_modify(). This
catches most of weired cobination of parameters to peer device
configuration.

Drop @link, @group and @index from iplink_parse() parameters list: they
are not needed outside.

While there change return -1 to exit(-1) for group parsing errors: we
want to stop further command processing unless -force option is given
to get error line easily.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/ip_common.h    |    3 +-
 ip/iplink.c       |  118 +++++++++++++++++++++++++----------------------------
 ip/iplink_vxcan.c |   13 +-----
 ip/link_veth.c    |   13 +-----
 4 files changed, 59 insertions(+), 88 deletions(-)

diff --git a/ip/ip_common.h b/ip/ip_common.h
index e4e628b..f762821 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -133,8 +133,7 @@ struct link_util {
 struct link_util *get_link_kind(const char *kind);
 
 int iplink_parse(int argc, char **argv, struct iplink_req *req,
-		 char **name, char **type, char **link, char **dev,
-		 int *group, int *index);
+		 char **name, char **type, char **dev);
 
 /* iplink_bridge.c */
 void br_dump_bridge_id(const struct ifla_bridge_id *id, char *buf, size_t len);
diff --git a/ip/iplink.c b/ip/iplink.c
index 4e9f571..e53d890 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -578,9 +578,9 @@ static void has_dev(const char *dev, int dev_index)
 }
 
 int iplink_parse(int argc, char **argv, struct iplink_req *req,
-		 char **name, char **type, char **link, char **dev,
-		 int *group, int *index)
+		 char **name, char **type, char **dev)
 {
+	char *link = NULL;
 	int ret, len;
 	char abuf[32];
 	int qlen = -1;
@@ -591,9 +591,10 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 	int numrxqueues = -1;
 	int dev_index = 0;
 	int link_netnsid = -1;
+	int index = 0;
+	int group = -1;
 	int addr_len = 0;
 
-	*group = -1;
 	ret = argc;
 
 	while (argc > 0) {
@@ -616,14 +617,14 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			}
 		} else if (strcmp(*argv, "index") == 0) {
 			NEXT_ARG();
-			if (*index)
+			if (index)
 				duparg("index", *argv);
-			*index = atoi(*argv);
-			if (*index <= 0)
+			index = atoi(*argv);
+			if (index <= 0)
 				invarg("Invalid \"index\" value", *argv);
 		} else if (matches(*argv, "link") == 0) {
 			NEXT_ARG();
-			*link = *argv;
+			link = *argv;
 		} else if (matches(*argv, "address") == 0) {
 			NEXT_ARG();
 			addr_len = ll_addr_a2n(abuf, sizeof(abuf), *argv);
@@ -816,10 +817,11 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 				  *argv, len);
 		} else if (strcmp(*argv, "group") == 0) {
 			NEXT_ARG();
-			if (*group != -1)
+			if (group != -1)
 				duparg("group", *argv);
-			if (rtnl_group_a2n(group, *argv))
+			if (rtnl_group_a2n(&group, *argv))
 				invarg("Invalid \"group\" value\n", *argv);
+			addattr32(&req->n, sizeof(*req), IFLA_GROUP, group);
 		} else if (strcmp(*argv, "mode") == 0) {
 			int mode;
 
@@ -946,80 +948,47 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 		}
 	}
 
-	return ret - argc;
-}
-
-static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
-{
-	char *dev = NULL;
-	char *name = NULL;
-	char *link = NULL;
-	char *type = NULL;
-	int index = 0;
-	int group;
-	struct link_util *lu = NULL;
-	struct iplink_req req = {
-		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
-		.n.nlmsg_flags = NLM_F_REQUEST | flags,
-		.n.nlmsg_type = cmd,
-		.i.ifi_family = preferred_family,
-	};
-	int ret;
-
-	ret = iplink_parse(argc, argv,
-			   &req, &name, &type, &link, &dev, &group, &index);
-	if (ret < 0)
-		return ret;
-
-	argc -= ret;
-	argv += ret;
-
-	if (!(flags & NLM_F_CREATE) && index) {
+	if (!(req->n.nlmsg_flags & NLM_F_CREATE) && index) {
 		fprintf(stderr,
 			"index can be used only when creating devices.\n");
 		exit(-1);
 	}
 
 	if (group != -1) {
-		if (dev)
-			addattr_l(&req.n, sizeof(req), IFLA_GROUP,
-					&group, sizeof(group));
-		else {
+		if (!*dev) {
 			if (argc) {
 				fprintf(stderr,
 					"Garbage instead of arguments \"%s ...\". Try \"ip link help\".\n",
 					*argv);
-				return -1;
+				exit(-1);
 			}
-			if (flags & NLM_F_CREATE) {
+			if (req->n.nlmsg_flags & NLM_F_CREATE) {
 				fprintf(stderr,
 					"group cannot be used when creating devices.\n");
-				return -1;
+				exit(-1);
 			}
 
-			addattr32(&req.n, sizeof(req), IFLA_GROUP, group);
-			if (rtnl_talk(&rth, &req.n, NULL) < 0)
-				return -2;
-			return 0;
+			*type = NULL;
+			goto out;
 		}
 	}
 
-	if (!(flags & NLM_F_CREATE)) {
-		if (!dev) {
+	if (!(req->n.nlmsg_flags & NLM_F_CREATE)) {
+		if (!*dev) {
 			fprintf(stderr,
 				"Not enough information: \"dev\" argument is required.\n");
 			exit(-1);
 		}
 
-		req.i.ifi_index = ll_name_to_index(dev);
-		if (!req.i.ifi_index)
-			return nodev(dev);
+		req->i.ifi_index = ll_name_to_index(*dev);
+		if (!req->i.ifi_index)
+			return nodev(*dev);
 
 		/* Not renaming to the same name */
-		if (name == dev)
-			name = NULL;
+		if (*name == *dev)
+			*name = NULL;
 	} else {
-		if (name != dev) {
+		if (*name != *dev) {
 			fprintf(stderr,
 				"both \"name\" and \"dev\" cannot be used when creating devices.\n");
 			exit(-1);
@@ -1031,18 +1000,39 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 			ifindex = ll_name_to_index(link);
 			if (!ifindex)
 				return nodev(link);
-			addattr_l(&req.n, sizeof(req), IFLA_LINK, &ifindex, 4);
+			addattr32(&req->n, sizeof(*req), IFLA_LINK, ifindex);
 		}
 
-		req.i.ifi_index = index;
+		req->i.ifi_index = index;
 	}
 
-	if (name) {
-		addattr_l(&req.n, sizeof(req),
-			  IFLA_IFNAME, name, strlen(name) + 1);
+	if (*name) {
+		addattr_l(&req->n, sizeof(*req),
+			  IFLA_IFNAME, *name, strlen(*name) + 1);
 	}
+out:
+	return ret - argc;
+}
+
+static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
+{
+	char *dev = NULL;
+	char *name = NULL;
+	char *type = NULL;
+	struct iplink_req req = {
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
+		.n.nlmsg_flags = NLM_F_REQUEST | flags,
+		.n.nlmsg_type = cmd,
+		.i.ifi_family = preferred_family,
+	};
+	int ret;
+
+	ret = iplink_parse(argc, argv, &req, &name, &type, &dev);
+	if (ret < 0)
+		return ret;
 
 	if (type) {
+		struct link_util *lu;
 		struct rtattr *linkinfo;
 		char *ulinep = strchr(type, '_');
 		int iflatype;
@@ -1056,6 +1046,10 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 			iflatype = IFLA_INFO_SLAVE_DATA;
 		else
 			iflatype = IFLA_INFO_DATA;
+
+		argc -= ret;
+		argv += ret;
+
 		if (lu && argc) {
 			struct rtattr *data;
 
diff --git a/ip/iplink_vxcan.c b/ip/iplink_vxcan.c
index d7e94a0..7d44e48 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -35,10 +35,7 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 {
 	char *dev = NULL;
 	char *name = NULL;
-	char *link = NULL;
 	char *type = NULL;
-	int index = 0;
-	int group;
 	int err;
 	struct rtattr *data;
 	struct ifinfomsg ifm_save, *ifm, *peer_ifm;
@@ -57,21 +54,13 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 	n->nlmsg_len += sizeof(struct ifinfomsg);
 
 	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
-			   &name, &type, &link, &dev, &group, &index);
+			   &name, &type, &dev);
 	if (err < 0)
 		return err;
 
 	if (type)
 		duparg("type", argv[err]);
 
-	if (name) {
-		addattr_l(n, 1024,
-			  IFLA_IFNAME, name, strlen(name) + 1);
-	}
-
-	if (group != -1)
-		addattr32(n, 1024, IFLA_GROUP, group);
-
 	peer_ifm = RTA_DATA(data);
 	memcpy(peer_ifm, ifm, sizeof(*ifm));
 	memcpy(ifm, &ifm_save, sizeof(*ifm));
diff --git a/ip/link_veth.c b/ip/link_veth.c
index b6d37b9..a42ea4e 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -33,10 +33,7 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 {
 	char *dev = NULL;
 	char *name = NULL;
-	char *link = NULL;
 	char *type = NULL;
-	int index = 0;
-	int group;
 	int err;
 	struct rtattr *data;
 	struct ifinfomsg ifm_save, *ifm, *peer_ifm;
@@ -55,21 +52,13 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 	n->nlmsg_len += sizeof(struct ifinfomsg);
 
 	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
-			   &name, &type, &link, &dev, &group, &index);
+			   &name, &type, &dev);
 	if (err < 0)
 		return err;
 
 	if (type)
 		duparg("type", argv[err]);
 
-	if (name) {
-		addattr_l(n, 1024,
-			  IFLA_IFNAME, name, strlen(name) + 1);
-	}
-
-	if (group != -1)
-		addattr32(n, 1024, IFLA_GROUP, group);
-
 	peer_ifm = RTA_DATA(data);
 	memcpy(peer_ifm, ifm, sizeof(*ifm));
 	memcpy(ifm, &ifm_save, sizeof(*ifm));
-- 
1.7.10.4