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
