On 04.11.2016 18:44, Joe Perches wrote:
> On Fri, 2016-11-04 at 11:07 -0400, David Miller wrote:
>> From: Lino Sanfilippo <lsan...@marvell.com>
>> > On 04.11.2016 07:53, Joe Perches wrote:
>> >> CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to
>> >> shortest
>> >> #446: FILE: drivers/net/ethernet/ethoc.c:446:
>> >> + int size = bd.stat >> 16;
>> >> + struct sk_buff *skb;
>> > should not this case be valid? Optically the longer line is already
>> > before the shorter.
>> > I think that the whole point in using this reverse xmas tree ordering
>> > is to have
>> > the code optically tidied up and not to enforce ordering between
>> > variable name lengths.
>>
>> That's correct.
>
> And also another reason the whole reverse xmas tree
> automatic declaration layout concept is IMO dubious.
>
> Basically, you're looking not at the initial ordering
> of automatics as important, but helping find a specific
> automatic when reversing from reading code is not always
> correct.
>
> Something like:
>
> static void function{args,...)
> {
> [longish list of reverse xmas tree identifiers...]
> struct foo *bar = longish_function(args, ...);
> struct foobarbaz *qux;
> [more identifers]
>
> [multiple screenfuls of code later...)
>
> new_function(..., bar, ...);
>
> [more code...]
> }
>
> and the reverse xmas tree helpfulness of looking up the
> type of bar is neither obvious nor easy.
>
In this case it is IMHO rather the declaration + initialization that makes
"bar" hard to find at one glance, not the use of RXT. You could do something
like
[longish list of reverse xmas tree identifiers...]
struct foobarbaz *qux;
struct foo *bar;
bar = longish_function(args, ...);
to increase readability.
Personally I find it more readable to always use a separate line for
initializations
by means of functions (regardless of whether the RXT scheme is used or not).
> My preference would be for a bar that serves coffee and alcohol.
>
At least a bar like this should not be too hard to find :)
Regards,
Lino
