lkml.org 
[lkml]   [2019]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [greybus-dev] [PATCH] Staging: greybus: usb: Fixed a coding style error
From
Date
On 3/31/19 1:40 AM, Joe Perches wrote:
> 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.

Then assign it where it's used. The point is we're talking about
a readability issue (long lines), and no matter how you try to fix
it there are tradeoffs, and it's subjective. In any case, I prefer
the use of the local variable to solve this readability problem over
splitting the line.

-Alex

\
 
 \ /
  Last update: 2019-03-31 08:44    [W:0.364 / U:0.576 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site