Re: Coding Style: Reverse XMAS tree declarations ? (was Re: [PATCH net-next v6 02/10] dpaa_eth: add support for DPAA Ethernet)
Joe Perches writes:
> On Thu, 2016-11-03 at 15:58 -0400, David Miller wrote:
>> From: Madalin Bucur
>> Date: Wed, 2 Nov 2016 22:17:26 +0200
>>
>> > This introduces the Freescale Data Path Acceleration Architecture
>> > +static inline size_t bpool_buffer_raw_size(u8 index, u8 cnt)
>> > +{
>> > + u8 i;
>> > + size_t res = DPAA_BP_RAW_SIZE / 2;
>>
>> Always order local variable declarations from longest to shortest line,
>> also know as Reverse Christmas Tree Format.
>
> I think this declaration sorting order is misguided but
> here's a possible change to checkpatch adding a test for it
> that does this test just for net/ and drivers/net/
And arch/powerpc too please.
cheers
Re: Coding Style: Reverse XMAS tree declarations ? (was Re: [PATCH net-next v6 02/10] dpaa_eth: add support for DPAA Ethernet)
On Fri, Nov 04, 2016 at 10:05:15AM -0700, Randy Dunlap wrote:
> On 11/03/16 23:53, Joe Perches wrote:
> > On Thu, 2016-11-03 at 15:58 -0400, David Miller wrote:
> >> From: Madalin Bucur
> >> Date: Wed, 2 Nov 2016 22:17:26 +0200
> >>
> >>> This introduces the Freescale Data Path Acceleration Architecture
> >>> +static inline size_t bpool_buffer_raw_size(u8 index, u8 cnt)
> >>> +{
> >>> + u8 i;
> >>> + size_t res = DPAA_BP_RAW_SIZE / 2;
> >>
> >> Always order local variable declarations from longest to shortest line,
> >> also know as Reverse Christmas Tree Format.
> >
> > I think this declaration sorting order is misguided but
> > here's a possible change to checkpatch adding a test for it
> > that does this test just for net/ and drivers/net/
>
> I agree with the misguided part.
> That's not actually in CodingStyle AFAICT. Where did this come from?
>
>
> thanks.
> --
> ~Randy
This puzzles me. The CodingStyle gives some pretty reasonable rationales
for coding style over above the "it's easier to read if it all looks the
same". I can see rationales for other approaches (and I am not proposing
any of these):
alphabetic orderEasier to search for declarations
complex to simple As in, structs and unions, pointers to simple
data (int, char), simple data. It seems like I
can deduce the simple types from usage, but more
complex I need to know things like the
particular structure.
group by usage Mirror the ontological locality in the code
Do we have a basis for thinking this is easier or more consistent than
any other approach?
--
David VL
Re: Coding Style: Reverse XMAS tree declarations ? (was Re: [PATCH net-next v6 02/10] dpaa_eth: add support for DPAA Ethernet)
On 11/03/16 23:53, Joe Perches wrote:
> On Thu, 2016-11-03 at 15:58 -0400, David Miller wrote:
>> From: Madalin Bucur
>> Date: Wed, 2 Nov 2016 22:17:26 +0200
>>
>>> This introduces the Freescale Data Path Acceleration Architecture
>>> +static inline size_t bpool_buffer_raw_size(u8 index, u8 cnt)
>>> +{
>>> + u8 i;
>>> + size_t res = DPAA_BP_RAW_SIZE / 2;
>>
>> Always order local variable declarations from longest to shortest line,
>> also know as Reverse Christmas Tree Format.
>
> I think this declaration sorting order is misguided but
> here's a possible change to checkpatch adding a test for it
> that does this test just for net/ and drivers/net/
I agree with the misguided part.
That's not actually in CodingStyle AFAICT. Where did this come from?
thanks.
--
~Randy
Re: Coding Style: Reverse XMAS tree declarations ? (was Re: [PATCH net-next v6 02/10] dpaa_eth: add support for DPAA Ethernet)
Hi, 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. Regards, Lino
