> -----Original Message----- > From: Richard Cochran [mailto:richardcoch...@gmail.com] > Sent: Tuesday, July 18, 2017 10:52 PM > To: Hangbin Liu <liuhang...@gmail.com> > Cc: Keller, Jacob E <jacob.e.kel...@intel.com>; Sushil Kulkarni > <sukul...@redhat.com>; linuxptp-devel@lists.sourceforge.net; Jiri Benc > <jb...@redhat.com> > Subject: Re: [Linuxptp-devel] [PATCHv2 4/9] rtnl: add function rtnl_link_info > > > You should return early when there are no resources to free: > > +int rtnl_link_info(struct interface *iface) > +{ > + int fd, index; > int err, fd, index; (See below for 'err') > > + index = if_nametoindex(iface->name); > + > + if (index == 0) { > + pr_err("failed to get interface %s index: %m", iface->name); > return -1; > + } > + > + fd = rtnl_open(); > + if (fd < 0) > return fd; > > + if (rtnl_link_query(fd, index)) > + goto no_info; > + if (rtnl_link_status(fd, NULL, iface->ts_iface)) > + goto no_info; > > Here you want to let the error code propagate: > > err = rtnl_link_query(fd, index); > if (err) { > goto no_info; > } > err = rtnl_link_status(fd, NULL, iface->ts_iface); > if (err) { > goto no_info; > } > > + /* If we do not have a slave, then use our own interface name > + * as ts_iface > + */ > + if (iface->ts_iface[0] == '\0') > + strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE); > > Move these two lines to the call site. They could be place in a > helper function in order to make the purpose clear without a comment. > See below... > > no_info: > rtnl_close(fd); > return err; > } > > > clock.c > ~~~~~~~ > > /* > * If we do not have a slave or the rtnl query failed, then use our > * own interface name as the time stamping interface name. > */ > static void ensure_ts_iface(iface) > { > if (iface->ts_iface[0] == '\0') > strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE); > } > > clock_create() > { > STAILQ_FOREACH(iface, &config->interfaces, list) { > rtnl_link_info(iface); > ensure_ts_iface(iface); > ... > } > }
Richard's solution is even better. Thanks, Jake ------------------------------------------------------------------------------ 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
