On Thu, Feb 28, 2019 at 09:22:22PM +0800, Hangbin Liu wrote:
> v3: a) Do not make rtnl_buf global as it may be freed by calling rtnl_close()
> while we are using it in rtnl_link_status()
This fix is wrong. You added a call to rtnl_close() for (AFAICT) no
good reason. That is the bug.
> int rtnl_link_status(int fd, char *device, rtnl_callback cb, void *ctx)
> {
> index = if_nametoindex(device);
> +
> + rtnl_buf = malloc(BUF_SIZE);
> if (!rtnl_buf) {
> - rtnl_len = 4096;
> - rtnl_buf = malloc(rtnl_len);
> - if (!rtnl_buf) {
> - pr_err("rtnl: low memory");
> - return -1;
The original code avoids calling malloc on every single invocation of
rtnl_link_status(), but you have removed that. This needs some kind
of justification.
> - }
> + pr_err("rtnl: low memory");
> + goto out;
> }
> +static int get_team_active_iface(int master_index)
> +{
> + struct rtattr *tb[TEAM_ATTR_MAX+1];
> + struct genlmsghdr *gnlh;
> + struct nlmsghdr *nlh;
> + char msg[BUF_SIZE];
> + int fd, gf_id;
> + int len = -1;
...
> +no_info:
> + rtnl_close(fd);
> + return len;
> +}
Here len is -1, as an error flag. Why simply propagate that error
correct up the call stack?
No need for rtnl_close() here.
Thanks,
Richard
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
