> -----Original Message-----
> From: Richard Alpe
> Sent: Tuesday, 12 April, 2016 03:45
> To: Jon Maloy; tipc-discussion@lists.sourceforge.net; Parthasarathy
> Bhuvaragan;
> Ying Xue
> Cc: ma...@donjonn.com
> Subject: Re: [PATCH net-next v2 3/5] tipc: refactor function
> tipc_link_timeout()
>
> On 2016-04-11 14:59, Jon Maloy wrote:
> > The function tipc_link_timeout() is unnecessary complex, and can
> > easily be made more readable.
> >
> > We do that with this commit. The only functional change is that we
> > remove a redundant test for whether the broadcast link is up or not.
> >
> > Signed-off-by: Jon Maloy <jon.ma...@ericsson.com>
> > ---
> > net/tipc/link.c | 36 ++++++++++++++++--------------------
> > 1 file changed, 16 insertions(+), 20 deletions(-)
> >
> > diff --git a/net/tipc/link.c b/net/tipc/link.c
> > index 238b125..774ad3c 100644
> > --- a/net/tipc/link.c
> > +++ b/net/tipc/link.c
> > @@ -704,37 +704,33 @@ static void link_profile_stats(struct tipc_link *l)
> > */
> > int tipc_link_timeout(struct tipc_link *l, struct sk_buff_head *xmitq)
> > {
> > - int rc = 0;
> > - int mtyp = STATE_MSG;
> > - bool xmit = false;
> > - bool prb = false;
> > + int mtyp, rc = 0;
> > + bool state = false;
> > + bool probe = false;
> > + bool setup = false;
> > u16 bc_snt = l->bc_sndlink->snd_nxt - 1;
> > u16 bc_acked = l->bc_rcvlink->acked;
> > - bool bc_up = link_is_up(l->bc_rcvlink);
> >
> > link_profile_stats(l);
> >
> > switch (l->state) {
> > case LINK_ESTABLISHED:
> > case LINK_SYNCHING:
> > - if (!l->silent_intv_cnt) {
> > - if (bc_up && (bc_acked != bc_snt))
> > - xmit = true;
> > - } else if (l->silent_intv_cnt <= l->abort_limit) {
> > - xmit = true;
> > - prb = true;
> > - } else {
> > - rc |= tipc_link_fsm_evt(l, LINK_FAILURE_EVT);
> > - }
> > - l->silent_intv_cnt++;
> > + if (l->silent_intv_cnt > l->abort_limit)
> > + return tipc_link_fsm_evt(l, LINK_FAILURE_EVT);
> > + mtyp = STATE_MSG;
> > + state = bc_acked != bc_snt;
> Didn't find anything about it in the coding style guide but I would
> have quoted the condition assignments like:
> state = (bc_acked != bc_snt);
Not sure. My general impression is that the preferred style is very
minimalistic when it comes to usage of parenthesis, assuming that the code
reader knows the operator precedence rules by heart.
E.g., it you end a function with "return (x == y);" checkpatch will tell you
that the parenthesis is redundant.
I think I'll keep it the way it is.
///jon
> > + probe = l->silent_intv_cnt;
> > + if (probe)
> > + l->silent_intv_cnt++;
> I think this is more readable:
> if (l->silent_intv_cnt)
> probe = l->silent_intv_cnt++;
>
> > break;
> > case LINK_RESET:
> > - xmit = l->rst_cnt++ <= 4;
> > - xmit |= !(l->rst_cnt % 16);
> > + setup = l->rst_cnt++ <= 4;
> > + setup |= !(l->rst_cnt % 16);
> > mtyp = RESET_MSG;
> > break;
> > case LINK_ESTABLISHING:
> > - xmit = true;
> > + setup = true;
> > mtyp = ACTIVATE_MSG;
> > break;
> > case LINK_PEER_RESET:
> > @@ -745,8 +741,8 @@ int tipc_link_timeout(struct tipc_link *l, struct
> sk_buff_head *xmitq)
> > break;
> > }
> >
> > - if (xmit)
> > - tipc_link_build_proto_msg(l, mtyp, prb, 0, 0, 0, xmitq);
> > + if (state || probe || setup)
> > + tipc_link_build_proto_msg(l, mtyp, probe, 0, 0, 0, xmitq);
> >
> > return rc;
> > }
> >
------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
