scripts: checkpatch.pl generate false positive errors for unnamed bitfields

[avolkov-1221]

Describe the bug

The current scripts/checkpatch.pl script give a false positive spacing error for the following, valid and correct, declaration:

typedef union some_register {
    struct {
        uint32_t a:3;
        uint32_t :3; /* reserved unnamed field */
        uint32_t b:3;
    };
    uint32_t some_reg;
} some_register_type_t;

And then checkpatch.pl will fail the check with error:

git show  | ./scripts/checkpatch.pl -

...............
-:21: ERROR:SPACING: space prohibited before that ':' (ctx:WxV)
#21: FILE: include/zephyr/unnamed_bitfield.h:7:
+       uint32_t  :3; /* reserved unnamed field */
                  ^
...............

To Reproduce

Just try to check the attached patch.

Expected behavior

The check should be passed without any errors.

Impact

This bug will block committing any C source file with empty bitfields necessary for the most of the peripherals hardware registers declarations.

Logs and console output

Environment (please complete the following information):

• OS: any
• Toolchain Zephyr SDK
• All Zephyr versions up to 3.7rc3

Additional context

Author: Andrey VOLKOV <andrey.volkov@munic.io>
Date:   Sat Jul 13 20:19:55 2024 +0200

    scripts: checkpatch.pl: Broken unnamed bitfields demo

    Signed-off-by: Andrey VOLKOV <andrey.volkov@munic.io>

diff --git a/include/zephyr/unnamed_bitfield.h b/include/zephyr/unnamed_bitfield.h
new file mode 100644
index 00000000000..6ac5f30e0d3
--- /dev/null
+++ b/include/zephyr/unnamed_bitfield.h
@@ -0,0 +1,13 @@
+#ifndef __UNNAMED_BITFIELD_H__
+#define __UNNAMED_BITFIELD_H__
+
+typedef union some_register {
+       struct {
+               uint32_t a:3;
+               uint32_t  :3; /* reserved unnamed field */
+               uint32_t b:3;
+       };
+       uint32_t some_reg;
+} some_register_type_t;
+
+#endif /* __UNNAMED_BITFIELD_H__*/

[henrikbrixandersen]

Why not just name the bitfields? Unnamed bitfields may be valid, but they are quite confusing IMHO.

[avolkov-1221]

If you'll have only one such field. If have tenth of them, then _r1..r1XX will not add readability. In any case it's 'false positive'.

Btw I've found that this code is passed:

    struct {
        unsigned int a:3;
        unsigned int :3; /* reserved unnamed field */
        unsigned int b:3;
    };

So, the problem is that the script treats uint32_t not as a type, but as a variable.

[cfriedt]

It would appear that unnamed bitfields are not disallowed by MISRA.

Length of a named signed bit-field is less than 2

https://help.klocwork.com/current/en-us/concepts/misraccheckerreference_2023_c11_nolinks_certified1.htm

They are definitely useful when defining in-memory structures, so I would support this bug being fixed in checkpatch.

[avolkov-1221]

It would appear that unnamed bitfields are not disallowed by MISRA.

Length of a named signed bit-field is less than 2

https://help.klocwork.com/current/en-us/concepts/misraccheckerreference_2023_c11_nolinks_certified1.htm

They are definitely useful when defining in-memory structures, so I would support this bug being fixed in checkpatch.

Btw I checked the latest version of the script on kernel.org, its behaviour is the same. So we should probably forward the bug report to them too.

[github-actions[bot]]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.