Re: [Linuxptp-devel] [PATCH 2/4] pmc_agent: Convert the method that queries the port properties.

[Richard Cochran]

On Mon, Nov 30, 2020 at 10:37:36AM -0800, Jacob Keller wrote:
> On 11/28/2020 9:08 AM, Richard Cochran wrote:

> >     LIST_FOREACH(p, &priv->ports, list) {
> > -           if (p->clock == clock) {
> > -                   ret = run_pmc_port_properties(priv->node, 1000, 
> > p->number,
> > -                                                 &state, &timestamping,
> > -                                                 iface);
> > -                   if (ret > 0)
> > -                           p->state = normalize_state(state);
> > +           if (p->clock != clock) {
> > +                   continue;
> > +           }
> 
> We do a continue now to skip over the clock until we found it. Ok.. Bit odd.

Right.  All I did was remove the IfOk anti-pattern (along with the
extra indentation).

> To me, this entire block might have read better as something like:
> 
> LIST_FOREACH(p, &priv->ports, list) {
>       if (p->clock == clock) {
>               break
>       }
> }
> 
> /* exit if we failed to find a clock */
> 
> /* do the run_pmc_port_properties */

Yes, that pattern would make more sense to me, but I've got bigger
fish to fry...

> None of this is really the fault of this patch, and could easily be left
> for a future cleanup/refactor. I believe the patch as written has the
> same semantics as the original before the return code cleanup.

+1

Thanks,
Richard


_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel