On Wed, Dec 30, 2020 at 02:49:07AM -0800, Cliff Spradlin via Linuxptp-devel wrote: > Previously, only the logical port number was emitted for most > port-related log messages. Now, the interface name is included. > > old: port 12: assuming the grand master role > new: port 12 (eth8): assuming the grand master role
I like this. Couple of comments, below... > diff --git a/clock.c b/clock.c > index 0156bc3..b30f2d1 100644 > --- a/clock.c > +++ b/clock.c > @@ -296,19 +296,20 @@ static int clock_fault_timeout(struct port *port, int > set) > struct fault_interval i; > > if (!set) { > - pr_debug("clearing fault on port %d", port_number(port)); > + pr_debug("clearing fault on port %d (%s)", port_number(port), > + port_name(port)); There are many, many log messages using the "port %d (%s)" format. Why not refactor them all to use a helper function? The new function could be called port2str(struct port *p) and live in port.c. See cid2str() in util.c as an example. You could have a series of two+ patches, with the first one introducing the helper, and the reset converting the call sites. > @@ -122,7 +123,8 @@ enum fsm_event e2e_event(struct port *p, int fd_index) > return EV_NONE; > > case FD_RTNL: > - pr_debug("port %hu: received link status notification", > portnum(p)); > + pr_debug("port %hu (%s): received link status notification", This adds a trailing space. There are a few more in this patch. Thanks, Richard _______________________________________________ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel