rusackas opened a new pull request, #20684:
URL: https://github.com/apache/nuttx/pull/20684
## Summary
* `nxstyle`'s "Left bracket not on separate line" check (the rule behind
the coding standard's ["Always on Separate
Lines"](https://nuttx.apache.org/docs/latest/contributing/coding_style.html#braces)
requirement for braces) only ever fired outside the file's
Private/Public Functions section. Inside a function body, the
declaration-nesting counter (`dnest`) is already nonzero by the time
the declaration's own opening brace is reached, because it gets set
as soon as the line's leading type keyword is matched, several
characters before the brace itself is seen. The check's `dnest == 0`
escape hatch therefore silently disabled the rule for every local
declaration, letting a brace like `const uint8_t xxx[] = {` sit on
the same line as the declaration with no warning, while the
identical pattern at file scope was (and still is) correctly flagged.
* That escape was only ever meant to exempt a *nested* initializer
brace, e.g. the per-entry `{ 1, 2 },` lines inside a file-scope array
of structs (`drivers/misc/dev_mem.c` has a good example), from
needing its own line. It was never supposed to exempt the first,
outer brace of a declaration's own initializer, and it only did so
by accident, because that's the one case where `dnest` happens to
already be nonzero when `bfunctions` is also true.
* Removes the escape for this one check so local declarations are held
to the same "brace on its own line" rule as global ones. Nested
initializer braces are unaffected, since those always start at the
line's own indent column and take the sibling `else if` branch below
this check, not this one.
* Fixes #16246 (local-variable array initializers weren't flagged,
only file-scope ones were).
* Fixes #16367 (nxstyle not catching badly nested/misaligned braces in
a function-local struct initializer; the originally reported
`drivers/sensors/nau7802.c` snippet is now caught, see Testing below).
* A related but separate report, #16249 (nxstyle not flagging upper-case
hex constants), is intentionally not addressed here: `raiden00pl`'s
comment on that issue notes it needs a new coding-standard rule added
first, which is a separate decision from this pure bug fix.
## Impact
* Is new feature added? NO.
* Is existing feature changed? YES. `nxstyle` now reports "Left bracket
not on separate line" for local declarations inside functions, matching
behavior it already had for file-scope declarations.
* Impact on user? YES, but narrowly: a future PR that introduces a local
declaration with this exact pattern will now get a real `nxstyle`/CI
`lint` failure where previously it silently passed. This is the
intended effect: the coding standard already prohibits this, `nxstyle`
just wasn't enforcing it inside functions.
* Impact on build? NO. `tools/nxstyle.c` is a host-side lint tool; it is
never compiled into, or run on, any NuttX target.
* Impact on hardware? NO.
* Impact on documentation? NO. This enforces the existing documented rule
("Braces always appear on a separate line containing nothing else other
than white space") more completely; it does not add or change a rule.
* Impact on security? NO.
* Impact on compatibility? NO build/runtime compatibility impact. A full
sweep of the tree (see Testing) found 44 pre-existing files that now
trip this check; since CI's `lint` job only runs `checkpatch.sh` over
each PR's own diff (not a whole-tree sweep), none of these will cause
an unrelated PR to fail. Left them alone here on purpose, since fixing
44 files across arch/boards/drivers is a separate, unrelated change
from this one per the "one functional change per PR" guideline; happy
to open follow-up PRs per-area, or let the relevant maintainers pick
them up, whichever is preferred. Full list below.
<details>
<summary>44 pre-existing files newly caught by this fix (not touched in this
PR)</summary>
```
arch/arm/src/common/stm32/stm32_adc_m0_v1.c:1441:42
arch/arm/src/cxd56xx/cxd56_nxaudio.c:1561:28
arch/arm/src/nrf91/nrf91_modem_gnss.c:748:23
arch/arm/src/ra4/ra_serial.c:406:26
arch/arm/src/samd5e5/sam_usb.c:6871:39
arch/arm64/src/bcm2711/bcm2711_mailbox.c:336:35
arch/mips/src/jz4780/jz4780_lcd.c:1524:35
arch/mips/src/jz4780/jz4780_lcd.c:1531:15
arch/risc-v/src/common/espressif/esp_usbserial.c:382:19
arch/risc-v/src/common/espressif/esp_usbserial.c:403:19
arch/risc-v/src/esp32c3-legacy/esp32c3_bignum.c:776:34
arch/risc-v/src/esp32c3-legacy/esp32c3_bignum.c:786:35
arch/risc-v/src/mpfs/mpfs_ddr.c:2723:31
arch/risc-v/src/mpfs/mpfs_ddr.c:2727:40
arch/risc-v/src/mpfs/mpfs_ddr.c:3202:29
arch/sim/src/sim/posix/sim_linuxspi.c:648:42
arch/xtensa/src/common/espressif/esp_openeth.c:443:26
arch/xtensa/src/common/espressif/esp_openeth.c:447:26
boards/arm/cxd56xx/drivers/sensors/cxd5610_gnss.c:1398:20
boards/arm/imx9/imx95-evk/src/imx95_i2c.c:68:41
boards/arm/imx9/mr-navq95b/src/imx95_i2c.c:68:41
boards/arm/imxrt/imxrt1170-evk/src/imxrt_flexspi_fram.c:196:22
boards/arm/imxrt/imxrt1170-evk/src/imxrt_flexspi_fram.c:309:37
boards/arm/imxrt/imxrt1170-evk/src/imxrt_flexspi_fram.c:346:37
boards/xtensa/esp32s3/common/src/esp32s3_lan9250.c:196:31
crypto/chachapoly.c:237:8
drivers/crypto/pnt/pnt_util.c:82:24
drivers/ioexpander/aw9523b.c:462:19
drivers/net/ksz9477.c:351:31
drivers/net/ksz9477.c:365:31
drivers/net/ksz9477.c:372:31
drivers/net/ksz9477.c:393:25
drivers/sensors/fakesensor_uorb.c:452:23
drivers/sensors/lis2mdl_uorb.c:254:32
drivers/sensors/lis2mdl_uorb.c:284:33
drivers/sensors/mcp9600_uorb.c:219:32
drivers/sensors/mcp9600_uorb.c:252:32
drivers/sensors/nau7802.c:127:32
drivers/sensors/nau7802.c:158:33
drivers/timers/rx8010.c:567:30
drivers/wireless/lpwan/sx126x/sx126x.c:928:23
drivers/wireless/lpwan/sx126x/sx126x.c:1251:53
drivers/wireless/lpwan/sx126x/sx126x.c:1262:56
sched/clock/clock_settime.c:69:35
```
</details>
## Testing
I confirm that changes are verified on local setup and work as intended:
* Build Host: macOS 27.0.1 (Darwin 27.0.0, arm64), Apple clang 21.0.0
(clang-2100.3.34.2).
* Target: `tools/nxstyle` itself, a host-side tool. Built exactly as
`checkpatch.sh`/`nxstyle_sweep.sh` build it: `cc -O2 -o nxstyle
tools/nxstyle.c`.
Minimal repro, before this change (local declaration, brace not flagged):
```c
void foo(void)
{
int i;
const unsigned char xxx[] = {
0x00, 0x00, 0x00, 0x00
};
i = xxx[0];
}
```
```
$ ./nxstyle local_brace.c
$ echo $?
0
```
Same file, after this change:
```
$ ./nxstyle local_brace.c
local_brace.c:7:30: error: Left bracket not on separate line
$ echo $?
1
```
The file-scope equivalent (control case) is unchanged by this patch,
flagged before and after:
```
$ ./nxstyle global_brace.c
global_brace.c:3:37: error: Left bracket not on separate line
```
The exact snippet originally reported in #16367
(`drivers/sensors/nau7802.c`, current `master`) is now caught:
```
$ ./nxstyle drivers/sensors/nau7802.c
drivers/sensors/nau7802.c:127:32: error: Left bracket not on separate line
drivers/sensors/nau7802.c:158:33: error: Left bracket not on separate line
```
Regression check: built both the unmodified and patched `nxstyle` and
ran each over every file `git ls-files` tracks under `*.c`/`*.h`
(18,004 files):
```
$ git ls-files -- '*.c' '*.h' | xargs -P 8 -n 1 ./nxstyle-before >
before.txt 2>&1
$ git ls-files -- '*.c' '*.h' | xargs -P 8 -n 1 ./nxstyle-after > after.txt
2>&1
$ comm -23 before_sorted.txt after_sorted.txt # lost diagnostics
(empty)
$ comm -13 before_sorted.txt after_sorted.txt | wc -l # new diagnostics
44
$ comm -13 before_sorted.txt after_sorted.txt | grep -vc "Left bracket not
on separate line"
0
```
Zero diagnostics lost (no regressions in any other check), 44 gained,
all of them the exact message this patch targets, listed above.
`checkpatch.sh` on the change itself:
```
$ ./tools/checkpatch.sh -f tools/nxstyle.c
✔️ All checks pass.
```
## PR verification Self-Check
* [x] This PR introduces only one functional change.
* [x] I have updated all required description fields above.
* [x] My PR adheres to Contributing
[Guidelines](https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md) and
[Documentation](https://nuttx.apache.org/docs/latest/contributing/index.html)
(git commit title and message, coding standard, etc).
* [ ] My PR is still work in progress (not ready for review).
* [x] My PR is ready for review and can be safely merged into a codebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
