netdev - Re: [PATCH] net/bonding: adjust codingstyle for bond

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20110510122228.GA3011@x61.tchesoft.com>
Date:	Tue, 10 May 2011 09:22:31 -0300
From:	Rafael Aquini <aquini@...ux.com>
To:	Jay Vosburgh <fubar@...ibm.com>
Cc:	kernel-janitors@...r.kernel.org,
	David Miller <davem@...emloft.net>,
	Joe Perches <joe@...ches.com>,
	Andy Gospodarek <andy@...yhouse.net>, shemminger@...tta.com,
	netdev@...r.kernel.org, Nicolas Kaiser <nikai@...ai.net>
Subject: Re: [PATCH] net/bonding: adjust codingstyle for bond_3ad files

Howdy Jay,

On Mon, May 09, 2011 at 07:00:40PM -0700, Jay Vosburgh wrote:
> >@@ -644,9 +640,7 @@ static void __update_ntt(struct lacpdu *lacpdu, struct port *port)
> >  */
> > static void __attach_bond_to_agg(struct port *port)
> > {
> >-	port = NULL; /* just to satisfy the compiler */
> >-	// This function does nothing since the parser/multiplexer of the receive
> >-	// and the parser/multiplexer of the aggregator are already combined
> >+	port = NULL;
> > }
> >
> > /**
> >@@ -659,9 +653,7 @@ static void __attach_bond_to_agg(struct port *port)
> >  */
> > static void __detach_bond_from_agg(struct port *port)
> > {
> >-	port = NULL; /* just to satisfy the compiler */
> >-	// This function does nothing sience the parser/multiplexer of the receive
> >-	// and the parser/multiplexer of the aggregator are already combined
> >+	port = NULL;
> 
> 	These two do-nothing functions should either
> 
> 	a) be removed entirely, and their call site replaced with a
> comment explaining this variation on the standard (that we don't need
> the "Attach_Mux_To_Aggregator" or "Detach_Mux_From_Aggregator" functions
> specified in the standard for the "Mux machine state diagram"), or
> 
> 	b) the comment needs to remain in some form to document this
> variation on the standard.
> 
Those comments were stripped from the function bodies because they already are
present in their leading comment blocks. Leaving them there sounded to me just
like echoing the very same thing twice.

However, each one of those functions is just called once from ad_mux_machine(),
so it might be a better idea (and easy) to wipe them away -- following your
suggestion a); or we can just leave them there, turning them into inline
functions instead. What approach do you prefer?

All other notes and suggestions you provide me will be accomplished soon.

Thanks for keep providing valuable feedback.

Cheers!
-- 
Rafael Aquini <aquini@...ux.com>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html