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 in iplink_vxcan.c and link_veth.c. Use memcpy() to save
peer device @ifm data structure as now ->ifi_index is set inside
iplist_parse(): it will be inlined by the compiler anyway.

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.popov...@gmail.com>
---
 ip/ip_common.h    |    3 +-
 ip/iplink.c       |  118 +++++++++++++++++++++++++----------------------------
 ip/iplink_vxcan.c |   27 +++---------
 ip/link_veth.c    |   27 +++---------
 4 files changed, 69 insertions(+), 106 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 ebe9e56..43c80e7 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -35,14 +35,10 @@ 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 err;
        struct rtattr *data;
-       int group;
-       struct ifinfomsg *ifm, *peer_ifm;
-       unsigned int ifi_flags, ifi_change;
+       struct ifinfomsg ifm_save, *ifm, *peer_ifm;
 
        if (strcmp(argv[0], "peer") != 0) {
                usage();
@@ -50,8 +46,8 @@ 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;
+       memcpy(&ifm_save, ifm, sizeof(*ifm));
+       ifm->ifi_index = 0;
        ifm->ifi_flags = 0;
        ifm->ifi_change = 0;
 
@@ -60,27 +56,16 @@ 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);
-       }
-
        peer_ifm = RTA_DATA(data);
-       peer_ifm->ifi_index = 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);
+       memcpy(peer_ifm, ifm, sizeof(*ifm));
+       memcpy(ifm, &ifm_save, sizeof(*ifm));
 
        addattr_nest_end(n, data);
        return argc - 1 - err;
diff --git a/ip/link_veth.c b/ip/link_veth.c
index a8e7cf7..bd8f36e 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -33,14 +33,10 @@ 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 err;
        struct rtattr *data;
-       int group;
-       struct ifinfomsg *ifm, *peer_ifm;
-       unsigned int ifi_flags, ifi_change;
+       struct ifinfomsg ifm_save, *ifm, *peer_ifm;
 
        if (strcmp(argv[0], "peer") != 0) {
                usage();
@@ -48,8 +44,8 @@ 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;
+       memcpy(&ifm_save, ifm, sizeof(*ifm));
+       ifm->ifi_index = 0;
        ifm->ifi_flags = 0;
        ifm->ifi_change = 0;
 
@@ -58,27 +54,16 @@ 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);
-       }
-
        peer_ifm = RTA_DATA(data);
-       peer_ifm->ifi_index = 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);
+       memcpy(peer_ifm, ifm, sizeof(*ifm));
+       memcpy(ifm, &ifm_save, sizeof(*ifm));
 
        addattr_nest_end(n, data);
        return argc - 1 - err;
-- 
1.7.10.4

Reply via email to