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(). Using memcpy() with constant length inlined by compiler. Drop @link, @group and @index from iplink_parse() parameters list: they are not needed outside. Signed-off-by: Serhey Popovych <serhe.popov...@gmail.com> --- ip/ip_common.h | 3 +- ip/iplink.c | 115 +++++++++++++++++++++++++---------------------------- ip/iplink_vxcan.c | 27 +++---------- ip/link_veth.c | 27 +++---------- 4 files changed, 68 insertions(+), 104 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 1adfaa0..9274a64 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"); return -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; } - 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; } - 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"); return -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"); return -1; @@ -1031,18 +1000,40 @@ 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); + addattr_l(&req->n, sizeof(*req), + IFLA_LINK, &ifindex, 4); } - 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 +1047,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