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 @name, @dev, @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.popov...@gmail.com>
---
 ip/ip_common.h    |    4 +-
 ip/iplink.c       |  143 ++++++++++++++++++++++++++---------------------------
 ip/iplink_vxcan.c |   23 +++------
 ip/link_veth.c    |   23 +++------
 4 files changed, 82 insertions(+), 111 deletions(-)

diff --git a/ip/ip_common.h b/ip/ip_common.h
index e4e628b..1b89795 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -132,9 +132,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);
+int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type);
 
 /* 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 6d3ebde..02042ed 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -569,10 +569,11 @@ static int iplink_parse_vf(int vf, int *argcp, char 
***argvp,
        return 0;
 }
 
-int iplink_parse(int argc, char **argv, struct iplink_req *req,
-                char **name, char **type, char **link, char **dev,
-                int *group, int *index)
+int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
 {
+       char *name = NULL;
+       char *dev = NULL;
+       char *link = NULL;
        int ret, len;
        char abuf[32];
        int qlen = -1;
@@ -583,9 +584,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) {
@@ -597,25 +599,25 @@ int iplink_parse(int argc, char **argv, struct iplink_req 
*req,
                        req->i.ifi_flags &= ~IFF_UP;
                } else if (strcmp(*argv, "name") == 0) {
                        NEXT_ARG();
-                       if (*name)
+                       if (name)
                                duparg("name", *argv);
                        if (check_ifname(*argv))
                                invarg("\"name\" not a valid ifname", *argv);
-                       *name = *argv;
-                       if (!*dev) {
-                               *dev = *name;
-                               dev_index = ll_name_to_index(*dev);
+                       name = *argv;
+                       if (!dev) {
+                               dev = name;
+                               dev_index = ll_name_to_index(dev);
                        }
                } 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);
@@ -661,8 +663,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req 
*req,
                                      generic, drv, offload))
                                exit(-1);
 
-                       if (offload && *name == *dev)
-                               *dev = NULL;
+                       if (offload && name == dev)
+                               dev = NULL;
                } else if (strcmp(*argv, "netns") == 0) {
                        NEXT_ARG();
                        if (netns != -1)
@@ -755,8 +757,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req 
*req,
                                return -1;
                        addattr_nest_end(&req->n, vflist);
 
-                       if (*name == *dev)
-                               *dev = NULL;
+                       if (name == dev)
+                               dev = NULL;
                } else if (matches(*argv, "master") == 0) {
                        int ifindex;
 
@@ -806,10 +808,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;
 
@@ -907,23 +910,25 @@ int iplink_parse(int argc, char **argv, struct iplink_req 
*req,
 
                        if (strcmp(*argv, "dev") == 0)
                                NEXT_ARG();
-                       if (*dev != *name)
+                       if (dev != name)
                                duparg2("dev", *argv);
                        if (check_ifname(*argv))
                                invarg("\"dev\" not a valid ifname", *argv);
-                       *dev = *argv;
-                       dev_index = ll_name_to_index(*dev);
+                       dev = *argv;
+                       dev_index = ll_name_to_index(dev);
                }
                argc--; argv++;
        }
 
+       ret -= argc;
+
        /* Allow "ip link add dev" and "ip link add name" */
-       if (!*name)
-               *name = *dev;
-       else if (!*dev)
-               *dev = *name;
-       else if (!strcmp(*name, *dev))
-               *name = *dev;
+       if (!name)
+               name = dev;
+       else if (!dev)
+               dev = name;
+       else if (!strcmp(name, dev))
+               name = dev;
 
        if (dev_index && addr_len) {
                int halen = nl_get_ll_addr_len(dev_index);
@@ -936,73 +941,40 @@ 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;
+                       return ret;
                }
        }
 
-       if (!(flags & NLM_F_CREATE)) {
+       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)
+               req->i.ifi_index = ll_name_to_index(dev);
+               if (!req->i.ifi_index)
                        return nodev(dev);
 
                /* Not renaming to the same name */
@@ -1021,18 +993,37 @@ 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),
+               addattr_l(&req->n, sizeof(*req),
                          IFLA_IFNAME, name, strlen(name) + 1);
        }
 
+       return ret;
+}
+
+static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
+{
+       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, &type);
+       if (ret < 0)
+               return ret;
+
        if (type) {
+               struct link_util *lu;
                struct rtattr *linkinfo;
                char *ulinep = strchr(type, '_');
                int iflatype;
@@ -1046,6 +1037,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 ebe9e56..8b08c9a 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -33,16 +33,11 @@ static void usage(void)
 static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
                          struct nlmsghdr *n)
 {
-       char *dev = NULL;
-       char *name = NULL;
-       char *link = NULL;
        char *type = NULL;
-       int index = 0;
        int err;
        struct rtattr *data;
-       int group;
        struct ifinfomsg *ifm, *peer_ifm;
-       unsigned int ifi_flags, ifi_change;
+       unsigned int ifi_flags, ifi_change, ifi_index;
 
        if (strcmp(argv[0], "peer") != 0) {
                usage();
@@ -52,35 +47,29 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, 
char **argv,
        ifm = NLMSG_DATA(n);
        ifi_flags = ifm->ifi_flags;
        ifi_change = ifm->ifi_change;
+       ifi_index = ifm->ifi_index;
        ifm->ifi_flags = 0;
        ifm->ifi_change = 0;
+       ifm->ifi_index = 0;
 
        data = addattr_nest(n, 1024, VXCAN_INFO_PEER);
 
        n->nlmsg_len += sizeof(struct ifinfomsg);
 
-       err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
-                          &name, &type, &link, &dev, &group, &index);
+       err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n, &type);
        if (err < 0)
                return err;
 
        if (type)
                duparg("type", argv[err]);
 
-       if (name) {
-               addattr_l(n, 1024,
-                         IFLA_IFNAME, name, strlen(name) + 1);
-       }
-
        peer_ifm = RTA_DATA(data);
-       peer_ifm->ifi_index = index;
+       peer_ifm->ifi_index = ifm->ifi_index;
        peer_ifm->ifi_flags = ifm->ifi_flags;
        peer_ifm->ifi_change = ifm->ifi_change;
        ifm->ifi_flags = ifi_flags;
        ifm->ifi_change = ifi_change;
-
-       if (group != -1)
-               addattr32(n, 1024, IFLA_GROUP, group);
+       ifm->ifi_index = ifi_index;
 
        addattr_nest_end(n, data);
        return argc - 1 - err;
diff --git a/ip/link_veth.c b/ip/link_veth.c
index a8e7cf7..33e8f2b 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -31,16 +31,11 @@ static void usage(void)
 static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
                          struct nlmsghdr *n)
 {
-       char *dev = NULL;
-       char *name = NULL;
-       char *link = NULL;
        char *type = NULL;
-       int index = 0;
        int err;
        struct rtattr *data;
-       int group;
        struct ifinfomsg *ifm, *peer_ifm;
-       unsigned int ifi_flags, ifi_change;
+       unsigned int ifi_flags, ifi_change, ifi_index;
 
        if (strcmp(argv[0], "peer") != 0) {
                usage();
@@ -50,35 +45,29 @@ static int veth_parse_opt(struct link_util *lu, int argc, 
char **argv,
        ifm = NLMSG_DATA(n);
        ifi_flags = ifm->ifi_flags;
        ifi_change = ifm->ifi_change;
+       ifi_index = ifm->ifi_index;
        ifm->ifi_flags = 0;
        ifm->ifi_change = 0;
+       ifm->ifi_index = 0;
 
        data = addattr_nest(n, 1024, VETH_INFO_PEER);
 
        n->nlmsg_len += sizeof(struct ifinfomsg);
 
-       err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
-                          &name, &type, &link, &dev, &group, &index);
+       err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n, &type);
        if (err < 0)
                return err;
 
        if (type)
                duparg("type", argv[err]);
 
-       if (name) {
-               addattr_l(n, 1024,
-                         IFLA_IFNAME, name, strlen(name) + 1);
-       }
-
        peer_ifm = RTA_DATA(data);
-       peer_ifm->ifi_index = index;
+       peer_ifm->ifi_index = ifm->ifi_index;
        peer_ifm->ifi_flags = ifm->ifi_flags;
        peer_ifm->ifi_change = ifm->ifi_change;
        ifm->ifi_flags = ifi_flags;
        ifm->ifi_change = ifi_change;
-
-       if (group != -1)
-               addattr32(n, 1024, IFLA_GROUP, group);
+       ifm->ifi_index = ifi_index;
 
        addattr_nest_end(n, data);
        return argc - 1 - err;
-- 
1.7.10.4

Reply via email to