Re: [Linuxptp-devel] [PATCHv2 4/9] rtnl: add function rtnl_link_info

[Richard C. <ric...@gm...>]

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);
		...
	}
}