On Sat, Jul 15, 2017 at 09:33:06PM +0800, Hangbin Liu wrote: > +static int rtnl_rtattr_parse(struct rtattr *tb[], int max, struct rtattr > *rta, int len) > +{ > + unsigned short type; > + > + memset(tb, 0, sizeof(struct rtattr *) * (max + 1));
This (max + 1) usage and ... > + while (RTA_OK(rta, len)) { > + type = rta->rta_type; > + if ((type <= max) && (!tb[type])) this <= test are a confusing style for array indexing. Instead, pass (tb[], int count) and use (type < count). But see below as well... > + tb[type] = rta; > + rta = RTA_NEXT(rta, len); > + } > + if (len) > + fprintf(stderr, "!!!Deficit %d, rta_len=%d\n", > + len, rta->rta_len); Use pr_err(). Also, the message doesn't make sense. Maybe use words like "length mismatch" or "unexpected length". > + return 0; > +} > + > +static int rtnl_linkinfo_parse(struct rtattr *rta, char *device) > +{ > + int index; > + struct rtattr *linkinfo[IFLA_INFO_MAX + 1]; > + struct rtattr *bond[IFLA_BOND_MAX + 1]; You allocate [MAX + 1], presumably to use the last element as list terminator. Yet the code never tests for the terminator. So why bother with the +1 ? > + rtnl_nested_rtattr_parse(linkinfo, IFLA_INFO_MAX, rta); > + > + if (linkinfo[IFLA_INFO_KIND]) { > + const char *kind = rta_getattr_str(linkinfo[IFLA_INFO_KIND]); Please place local variable at the top of the function, and use reverse Christmas tree style. > + if (kind && !strncmp(kind, "bond", 4) && > + linkinfo[IFLA_INFO_DATA]) { > + rtnl_nested_rtattr_parse(bond, IFLA_BOND_MAX, > + linkinfo[IFLA_INFO_DATA]); > + > + if (bond[IFLA_BOND_ACTIVE_SLAVE]) { > + index = > rta_getattr_u32(bond[IFLA_BOND_ACTIVE_SLAVE]); > + > + if (!if_indextoname(index, device)) { > + pr_err("failed to get device name: %m"); > + return -1; > + } > + } > + } > + } > + return 0; > +} > + > int rtnl_link_status(int fd, rtnl_callback cb, void *ctx) > { > int index, len; > @@ -92,6 +151,18 @@ int rtnl_link_status(int fd, rtnl_callback cb, void *ctx) > struct msghdr msg; > struct nlmsghdr *nh; > struct ifinfomsg *info = NULL; > + char *device; > + struct rtattr *tb[IFLA_MAX+1]; > + > + if (cb) > + device = calloc(1, sizeof(MAX_IFNAME_SIZE + 1)); > + else > + device = (char *)ctx; > + > + if (!device) { > + fprintf(stderr, "rtnl: no enought memory for device name\n"); Use pr_err(), and enought is misspelled. How about this: if (!device) { pr_err("rtnl: low memory"); } > + return -1; > + } > > if (!rtnl_buf) { > rtnl_len = 4096; Thanks, Richard ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel