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

Reply via email to