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/ (this likely won't apply because Evolution is a horrible email client for sending patches, but the attachment should) An example output: $ ./scripts/checkpatch.pl -f drivers/net/ethernet/ethoc.c --types=reverse_xmas_tree --show-types CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to shortest #200: FILE: drivers/net/ethernet/ethoc.c:200: + void __iomem *iobase; + void __iomem *membase; CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to shortest #202: FILE: drivers/net/ethernet/ethoc.c:202: + int dma_alloc; + resource_size_t io_region_size; CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to shortest #305: FILE: drivers/net/ethernet/ethoc.c:305: + int i; + void *vma; 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; CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to shortest #515: FILE: drivers/net/ethernet/ethoc.c:515: + int count; + struct ethoc_bd bd; CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to shortest #675: FILE: drivers/net/ethernet/ethoc.c:675: + struct ethoc *priv = netdev_priv(dev); + struct phy_device *phy; CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to shortest #756: FILE: drivers/net/ethernet/ethoc.c:756: + struct ethoc *priv = netdev_priv(dev); + struct mii_ioctl_data *mdio = if_mii(ifr); CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to shortest #801: FILE: drivers/net/ethernet/ethoc.c:801: + u32 mode = ethoc_read(priv, MODER); + struct netdev_hw_addr *ha; CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to shortest #996: FILE: drivers/net/ethernet/ethoc.c:996: + struct resource *res = NULL; + struct resource *mmio = NULL; CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to shortest #1001: FILE: drivers/net/ethernet/ethoc.c:1001: + int ret = 0; + bool random_mac = false; CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to shortest #1002: FILE: drivers/net/ethernet/ethoc.c:1002: + bool random_mac = false; + struct ethoc_platform_data *pdata = dev_get_platdata(&pdev->dev); total: 0 errors, 0 warnings, 11 checks, 1297 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. drivers/net/ethernet/ethoc.c has style problems, please review. NOTE: Used message types: REVERSE_XMAS_TREE NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. ---  scripts/checkpatch.pl | 70 ++++++++++++++++++++++++++++++++++++++++-----------  1 file changed, 56 insertions(+), 14 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index fdacd759078e..dd344ac77cb8 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2114,6 +2114,29 @@ sub pos_last_openparen {   return length(expand_tabs(substr($line, 0, $last_openparen))) + 1;  }   +sub get_decl { + my ($line) = @_; + + # typical declarations + if ($line =~ /^\+\s+($Declare\s*$Ident)\s*[=,;:\[]/) { + return $1; + } + # function pointer declarations + if ($line =~ /^\+\s+($Declare\s*\(\s*\*\s*$Ident\s*\))\s*[=,;:\[\(]/) { + return $1; + } + # foo bar; where foo is some local typedef or #define + if ($line =~ /^\+\s+($Ident(?:\s+|\s*\*\s*)$Ident)\s*[=,;\[]/) { + return $1; + } + # known declaration macros + if ($line =~ /^\+\s+$(declaration_macros)/) { + return $1; + } + + return undef; +} +  sub process {   my $filename = shift;   @@ -3063,9 +3086,7 @@ sub process {   $last_blank_line = $linenr;   }   -# check for missing blank lines after declarations - if ($sline =~ /^\+\s+\S/ && #Not at char 1 - # actual declarations + my $prev_decl =       ($prevline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ ||   # function pointer declarations        $prevline =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || @@ -3078,25 +3099,30 @@ sub process {   # other possible extensions of declaration lines         $prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ ||   # not starting a section or a macro "\" extended line -       $prevline =~ /(?:\{\s*|\\)$/) && - # looks like a declaration -     !($sline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || +       $prevline =~ /(?:\{\s*|\\)$/); + my $sline_decl = +     $sline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ ||   # function pointer declarations -       $sline =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || +     $sline =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ ||   # foo bar; where foo is some local typedef or #define -       $sline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || +     $sline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ ||   # known declaration macros -       $sline =~ /^\+\s+$declaration_macros/ || +     $sline =~ /^\+\s+$declaration_macros/ ||   # start of struct or union or enum -       $sline =~ /^\+\s+(?:union|struct|enum|typedef)\b/ || +     $sline =~ /^\+\s+(?:union|struct|enum|typedef)\b/ ||   # start or end of block or continuation of declaration -       $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ || +     $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ ||   # bitfield continuation -       $sline =~ /^\+\s+$Ident\s*:\s*\d+\s*[,;]/ || +     $sline =~ /^\+\s+$Ident\s*:\s*\d+\s*[,;]/ ||   # other possible extensions of declaration lines -       $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/) && +     $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/; + +# check for missing blank lines after declarations + if ($sline =~ /^\+\s+\S/ && #Not at char 1 + # actual declarations +     $prev_decl && !$sline_decl &&   # indentation of previous and current line are the same -     (($prevline =~ /\+(\s+)\S/) && $sline =~ /^\+$1\S/)) { +     ($prevline =~ /\+(\s+)\S/ && $sline =~ /^\+$1\S/)) {   if (WARN("LINE_SPACING",    "Missing a blank line after declarations\n" . $hereprev) &&       $fix) { @@ -3104,6 +3130,22 @@ sub process {   }   }   +# check for reverse christmas tree declarations in net/ and drivers/net/ + if ($realfile =~ m@^(?:drivers/net/|net/)@ && +     $sline =~ /^\+\s+\S/ && #Not at char 1 + # actual declarations +     $prev_decl && $sline_decl && + # indentation of previous and current line are the same +     (($prevline =~ /\+(\s+)\S/) && $sline =~ /^\+$1\S/)) { + my $p = get_decl($prevline); + my $l = get_decl($sline); + if (defined($p) && defined($l) && length($p) < length($l) && +     CHK("REVERSE_XMAS_TREE", + "Prefer ordering declarations longest to shortest\n" . $hereprev) && +     $fix) { + } + } +  # check for spaces at the beginning of a line.  # Exceptions:  #  1) within comments