netdev - RE: [net-next 05/16] i40e: fix long lines

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <063D6719AE5E284EB5DD2968C1650D6D457950@AcuExch.aculab.com>
Date:	Fri, 10 Jan 2014 10:02:27 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Joe Perches' <joe@...ches.com>,
	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
CC:	"davem@...emloft.net" <davem@...emloft.net>,
	Mitch Williams <mitch.a.williams@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>,
	Jesse Brandeburg <jesse.brandeburg@...el.com>
Subject: RE: [net-next 05/16] i40e: fix long lines

> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c
> b/drivers/net/ethernet/intel/i40e/i40e_common.c
> []
> > @@ -681,7 +681,8 @@ aq_add_vsi_exit:
> >   * @cmd_details: pointer to command details structure or NULL
> >   **/
> >  i40e_status i40e_aq_set_vsi_unicast_promiscuous(struct i40e_hw *hw,
> > -				u16 seid, bool set, struct i40e_asq_cmd_details *cmd_details)
> > +				u16 seid, bool set,
> > +				struct i40e_asq_cmd_details *cmd_details)
> 
> Another way this could be changed is to put the
> return value on a separate line like:
> 
> i40e_status
> i40e_aq_set_vsi_unicast_promiscuous(struct i40e_hw *hw, u16 seid, bool set,
> 				    struct i40e_asq_cmd_details *cmd_details)

Personally I prefer that so I can find the definition by grepping
for '^function_name' - but it doesn't seem to be done in any linux
source files.

> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_prototype.h
> b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
> []
> > @@ -93,9 +93,9 @@ i40e_status i40e_aq_set_vsi_broadcast(struct i40e_hw *hw,
> >  				u16 vsi_id, bool set_filter,
> >  				struct i40e_asq_cmd_details *cmd_details);
> >  i40e_status i40e_aq_set_vsi_unicast_promiscuous(struct i40e_hw *hw,
> > -				u16 vsi_id, bool set, struct i40e_asq_cmd_details *cmd_details);
> > +		u16 vsi_id, bool set, struct i40e_asq_cmd_details *cmd_details);
> 
> i40e_status
> i40e_aq_set_vsi_unicast_promiscuous(struct i40e_hw *hw, u16 vsi_id, bool set,
> 				    struct i40e_asq_cmd_details *cmd_details);
> 
> etc...
> 
> but once you use extremely long 35+ characters
> function names, 80 columns gets a bit silly too.

Again this only a real problem when the coding stand requires that
continuation lines have the parameters lined up under the '('.
If they are indented by 4 spaces (as many of the BSDs do) then you
do stand a chance of laying out a call in a reasonable number of
vertical lines.

My Actual preferred layout is to exclude tabs, indent by 4 spaces
and double-indent continuation lines.
I'll also start continuation lines (of expressions) with an operator
to make it even clearer they are continuation lines.
When I'm scan-reading code I tend to concentrate on the LHS of each line.
- so that is where the information need to be.

Not that I expect Linux to change!
(Other code bases have much worse layout rules.)

	David



--
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