On Sat, Aug 05, 2017 at 10:31:48AM +0200, Richard Cochran wrote:
> On Sat, Jul 15, 2017 at 09:33:06PM +0800, Hangbin Liu wrote:
> > @@ -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;
>
> The meaning of 'ctx' changes according to the value of 'cb'. This
> overloading of the function arguments is gross.
Yes, this is kind of ugly.
> Please find another way to structure this. You only have two call
> paths for rtnl_link_status:
>
> 1. port_event() -> rtnl_link_status()
The goal of this call path is to call port_link_status() whenever link status
changed. And the call path should like
1. port_event() -> rtnl_link_status() -> port_link_status()
> 2. clock_create() -> rtnl_link_info() -> rtnl_link_status()
This one only want to get the bond slave info when add iface at the begining.
Ironically, they share the most code...
I tried to add two wrapper function like
int rtnl_link_info(int fd, char *index, char *up, char *device)
{
/* set index, link up status and device here */
}
void rtnl_link_status(int fd, rtnl_callback cb, void *ctx)
{
rtnl_link_info(fd, &index, &up, device);
cb(ctx, index, up, device);
}
static int rtnl_get_interface_slave(int fd, char *device)
{
return rtnl_link_info(fd, &index, &up, device);
}
But this is incorrect becuase we need call port_link_status() every time
nh->nlmsg_type == RTM_NEWLINK.
for ( ; NLMSG_OK(nh, len); nh = NLMSG_NEXT(nh, len)) {
if (nh->nlmsg_type == RTM_NEWLINK) {
cb(ctx, index, info->ifi_flags & IFF_RUNNING ? 1 : 0, device);
}
}
So now I only have one workaround: add one more parameter...
int rtnl_link_status(int fd, char *device, rtnl_callback cb, void *ctx)
Then if we want to get device name via rtnl_link_info(), we could call like
rtnl_link_status(fd, iface->ts_label, NULL, NULL);
If we want to callback functions, we call it like
rtnl_link_status(fd, NULL, port_link_status, p);
What do you think? any other good ideas?
Thanks
Hangbin
------------------------------------------------------------------------------
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
