Skip to content

Commit 3c420a4

Browse files
carlescufikartben
authored andcommitted
checkpatch: Adapt the braces check to Zephyr
scripts/checkpatch.pl was written originally for the Linux kernel, and its code reflects the kernel's coding style. In particular, it has checks for unneeded braces around single-statement if/else/for/while conditions. In Zephyr however, braces are always required, and so the checks needed modifying to verify the opposite condition. In order to enable the now-compatible checks, we also remove the --ignore BRACES statement in .checkpatch.conf. Limitations: the current code works well if there are not conditional statements (e.g. #if, #ifdef or #endif) next to the if/else/for/while conditions. This is rarely the case, but triggers with the Bluetooth controller in code like this: ``` #if defined(CONFIG_BT_PERIPHERAL) if (!lll->is_hdcd) #endif /* CONFIG_BT_PERIPHERAL */ { ``` ``` } else #endif /* CONFIG_BT_CTLR_PRIVACY */ { ``` ``` #if defined(CONFIG_BT_CTLR_DF_ADV_CTE_TX) if (lll->cte_started) { radio_switch_complete(phy_s, 0, phy_s, 0); } else #endif /* CONFIG_BT_CTLR_DF_ADV_CTE_TX */ { ``` ``` #ifdef DUAL_BANK while ((FLASH_STM32_REGS(dev)->SR1 & FLASH_SR_QW) || (FLASH_STM32_REGS(dev)->SR2 & FLASH_SR_QW)) #else while (FLASH_STM32_REGS(dev)->SR1 & FLASH_SR_QW) #endif { ``` Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
1 parent 0f948fd commit 3c420a4

File tree

3 files changed

+32
-60
lines changed

3 files changed

+32
-60
lines changed

.checkpatch.conf

-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
--min-conf-desc-length=1
66
--typedefsfile=scripts/checkpatch/typedefsfile
77

8-
--ignore BRACES
98
--ignore PRINTK_WITHOUT_KERN_LEVEL
109
--ignore SPLIT_STRING
1110
--ignore VOLATILE

doc/contribute/guidelines.rst

+1-2
Original file line numberDiff line numberDiff line change
@@ -545,8 +545,7 @@ exceptions:
545545
* The line length is 100 columns or fewer. In the documentation, longer lines
546546
for URL references are an allowed exception.
547547
* Add braces to every ``if``, ``else``, ``do``, ``while``, ``for`` and
548-
``switch`` body, even for single-line code blocks. Use the ``--ignore BRACES``
549-
flag to make *checkpatch* stop complaining.
548+
``switch`` body, even for single-line code blocks.
550549
* Use spaces instead of tabs to align comments after declarations, as needed.
551550
* Use C89-style single line comments, ``/* */``. The C99-style single line
552551
comment, ``//``, is not allowed.

scripts/checkpatch.pl

+31-57
Original file line numberDiff line numberDiff line change
@@ -5497,8 +5497,9 @@ sub process {
54975497
my ($whitespace) = ($cond =~ /^((?:\s*\n[+-])*\s*)/s);
54985498
my $offset = statement_rawlines($whitespace) - 1;
54995499

5500-
$allowed[$allow] = 0;
5501-
#print "COND<$cond> whitespace<$whitespace> offset<$offset>\n";
5500+
# always allow braces (i.e. require them)
5501+
$allowed[$allow] = 1;
5502+
#print "COND<$cond> block<$block> whitespace<$whitespace> offset<$offset>\n";
55025503

55035504
# We have looked at and allowed this specific line.
55045505
$suppress_ifbraces{$ln + $offset} = 1;
@@ -5507,46 +5508,37 @@ sub process {
55075508
$ln += statement_rawlines($block) - 1;
55085509

55095510
substr($block, 0, length($cond), '');
5511+
#print "COND<$cond> block<$block>\n";
5512+
5513+
# Remove any 0x1C characters. The script replaces
5514+
# comments /* */ with those.
5515+
$block =~ tr/\x1C//d;
55105516

55115517
$seen++ if ($block =~ /^\s*{/);
55125518

5513-
#print "cond<$cond> block<$block> allowed<$allowed[$allow]>\n";
5514-
if (statement_lines($cond) > 1) {
5515-
#print "APW: ALLOWED: cond<$cond>\n";
5516-
$allowed[$allow] = 1;
5517-
}
5518-
if ($block =~/\b(?:if|for|while)\b/) {
5519-
#print "APW: ALLOWED: block<$block>\n";
5520-
$allowed[$allow] = 1;
5521-
}
5522-
if (statement_block_size($block) > 1) {
5523-
#print "APW: ALLOWED: lines block<$block>\n";
5524-
$allowed[$allow] = 1;
5525-
}
55265519
$allow++;
55275520
}
5528-
if ($seen) {
5529-
my $sum_allowed = 0;
5530-
foreach (@allowed) {
5531-
$sum_allowed += $_;
5532-
}
5533-
if ($sum_allowed == 0) {
5534-
WARN("BRACES",
5535-
"braces {} are not necessary for any arm of this statement\n" . $herectx);
5536-
} elsif ($sum_allowed != $allow &&
5537-
$seen != $allow) {
5538-
CHK("BRACES",
5539-
"braces {} should be used on all arms of this statement\n" . $herectx);
5540-
}
5521+
my $sum_allowed = 0;
5522+
foreach (@allowed) {
5523+
$sum_allowed += $_;
5524+
}
5525+
#print "sum_allowed<$sum_allowed> seen<$seen> allow<$allow>\n";
5526+
if ($sum_allowed == 0) {
5527+
WARN("BRACES",
5528+
"braces {} are not necessary for any arm of this statement\n" . $herectx);
5529+
} elsif ($seen != $allow) {
5530+
WARN("BRACES",
5531+
"braces {} must be used on all arms of this statement\n" . $herectx);
55415532
}
55425533
}
55435534
}
55445535
if (!defined $suppress_ifbraces{$linenr - 1} &&
55455536
$line =~ /\b(if|while|for|else)\b/) {
55465537
my $allowed = 0;
5547-
5548-
# Check the pre-context.
5549-
if (substr($line, 0, $-[0]) =~ /(\}\s*)$/) {
5538+
# Check the pre-context for:
5539+
# '#': #if
5540+
# '}': } while()
5541+
if (substr($line, 0, $-[0]) =~ /([\#\}]\s*)$/) {
55505542
#print "APW: ALLOWED: pre<$1>\n";
55515543
$allowed = 1;
55525544
}
@@ -5556,39 +5548,21 @@ sub process {
55565548

55575549
# Check the condition.
55585550
my ($cond, $block) = @{$chunks[0]};
5559-
#print "CHECKING<$linenr> cond<$cond> block<$block>\n";
5551+
#print "level $level CHECKING<$linenr> cond<$cond> block<$block>\n";
55605552
if (defined $cond) {
55615553
substr($block, 0, length($cond), '');
55625554
}
5563-
if (statement_lines($cond) > 1) {
5564-
#print "APW: ALLOWED: cond<$cond>\n";
5565-
$allowed = 1;
5566-
}
5567-
if ($block =~/\b(?:if|for|while)\b/) {
5568-
#print "APW: ALLOWED: block<$block>\n";
5569-
$allowed = 1;
5570-
}
5571-
if (statement_block_size($block) > 1) {
5572-
#print "APW: ALLOWED: lines block<$block>\n";
5573-
$allowed = 1;
5574-
}
5575-
# Check the post-context.
5576-
if (defined $chunks[1]) {
5577-
my ($cond, $block) = @{$chunks[1]};
5578-
if (defined $cond) {
5579-
substr($block, 0, length($cond), '');
5580-
}
5581-
if ($block =~ /^\s*\{/) {
5582-
#print "APW: ALLOWED: chunk-1 block<$block>\n";
5583-
$allowed = 1;
5584-
}
5585-
}
5586-
if ($level == 0 && $block =~ /^\s*\{/ && !$allowed) {
5555+
# Remove any 0x1C characters. The script replaces
5556+
# comments /* */ with those.
5557+
$block =~ tr/\x1C//d;
5558+
#print sprintf '%v02X', $block;
5559+
#print "\n";
5560+
if ($level == 0 && $block !~ /^\s*\{/ && !$allowed) {
55875561
my $cnt = statement_rawlines($block);
55885562
my $herectx = get_stat_here($linenr, $cnt, $here);
55895563

55905564
WARN("BRACES",
5591-
"braces {} are not necessary for single statement blocks\n" . $herectx);
5565+
"braces {} are required around if/while/for/else\n" . $herectx);
55925566
}
55935567
}
55945568

0 commit comments

Comments
 (0)