linux-kernel - Re: [greybus-dev] [PATCH] Staging: greybus: usb: Fixed a coding style error

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: <8b6afee25fe6d2248353015fe2303d793b2a1d49.camel@perches.com>
Date:   Sat, 30 Mar 2019 23:40:28 -0700
From:   Joe Perches <joe@...ches.com>
To:     Alex Elder <elder@...aro.org>,
        Will Cunningham <wjcunningham7@...il.com>, johan@...nel.org
Cc:     devel@...verdev.osuosl.org, elder@...nel.org,
        linux-kernel@...r.kernel.org, greybus-dev@...ts.linaro.org
Subject: Re: [greybus-dev] [PATCH] Staging: greybus: usb: Fixed a coding
 style error

On Sun, 2019-03-31 at 01:20 -0500, Alex Elder wrote:
> On 3/31/19 1:04 AM, Joe Perches wrote:
> > Blind adherence to 80 column limits leads to poor looking
> > code.  Especially with longish identifier lengths.
> I agree.  If it were me, I'd use a local variable.  For example:
> 
> 	struct greybus_descriptor_cport *cport_desc = gbphy_dev->cport_desc;
> 	...
> 	connection = gb_connection_create(gbphy_dev->bundle,
> 					  le16_to_cpu(cport_desc->id), NULL);
> 
> Or maybe better:
> 
> 	u16 cport_id = le16_to_cpu(gbphy_dev->cport_desc->id);
> 	...
> 	connection = gb_connection_create(gbphy_dev->bundle, cport_id, NULL);

True.

A possible negative though:

Temporaries that are only used once are sometimes
less readable as the declaration is supposed to be
done at an open brace and that could be relatively
far away from the set and use.