-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Description
Describe the bug
The weekly CI run shows an summary of an error below, with 270 hits.
error: expected identifier or '(' before numeric constantThe posix_limits.h change went in just a few days ago in #94612. However, it's hard to call this a regression per se, since this structure (and possibly others in the HAL) use a standard macro as a field name.
So this has been a bug that has been hidden for some time. It looks like the file was first added in zephyrproject-rtos/hal_nxp@a8ac3f6 .
There was a similar issue about 1 year ago with the STM HAL, and of course PAGE_SIZE or PAGESIZE has a common semantic meaning, so it's understandable.
I have tried surrounding PAGESIZE with #ifndef inside of posix_limits.h but since the offending entity in this case is a field in a struct, that does not help.
target platform: verdin_imx8mp/mimx8ml8/m7
diagnosis: a bug in the NXP HAL
regression: ab8b55a
Regression
- This is a regression.
Steps to reproduce
west build -p auto -b verdin_imx8mp/mimx8ml8/m7 tests/lib/c_lib/common -T libraries.libc.common.minimal- See error
More relevant error information is available by substituting CONFIG_POSIX_PAGE_SIZE with a constant (e.g. 0x40).
diff --git a/include/zephyr/posix/posix_limits.h b/include/zephyr/posix/posix_limits.h
index 9c225b4b4d4..ee70f346cef 100644
--- a/include/zephyr/posix/posix_limits.h
+++ b/include/zephyr/posix/posix_limits.h
@@ -91,8 +91,8 @@
COND_CODE_1(CONFIG_POSIX_MESSAGE_PASSING, (CONFIG_POSIX_MQ_OPEN_MAX), (0))
#define MQ_PRIO_MAX _POSIX_MQ_PRIO_MAX
#define OPEN_MAX CONFIG_POSIX_OPEN_MAX
-#define PAGE_SIZE CONFIG_POSIX_PAGE_SIZE
-#define PAGESIZE CONFIG_POSIX_PAGE_SIZE
+#define PAGE_SIZE 0x40
+#define PAGESIZE 0x40
#define PATH_MAX _POSIX_PATH_MAX
#define PTHREAD_DESTRUCTOR_ITERATIONS _POSIX_THREAD_DESTRUCTOR_ITERATIONS
#define PTHREAD_KEYS_MAX \In file included from /home/cfriedt/zephyrproject/zephyr/lib/libc/minimal/include/limits.h:85,
from /home/cfriedt/zephyrproject/zephyr/include/zephyr/kernel_includes.h:22,
from /home/cfriedt/zephyrproject/zephyr/include/zephyr/kernel.h:17,
from /home/cfriedt/zephyrproject/zephyr/tests/lib/c_lib/common/src/main.c:21:
/home/cfriedt/zephyrproject/modules/hal/nxp/mcux/mcux-sdk-ng/devices/i.MX/i.MX8MP/periph/PERI_USB.h:171:17: error: expected identifier or '(' before numeric constant
171 | __I uint32_t PAGESIZE; /**< Page Size Register, offset: 0x28 */
| ^~~~~~~~
ninja: build stopped: subcommand failed.Relevant log output
In file included from <command-line>:
/__w/zephyr/zephyr/twister-out/verdin_imx8mp_mimx8ml8_m7/zephyr/tests/lib/c_lib/common/libraries.libc.common.minimal/zephyr/include/generated/zephyr/autoconf.h:351:32: error: expected identifier or '(' before numeric constant
351 | #define CONFIG_POSIX_PAGE_SIZE 0x40
| ^~~~
/__w/zephyr/zephyr/include/zephyr/posix/posix_limits.h:93:39: note: in expansion of macro 'CONFIG_POSIX_PAGE_SIZE'
93 | #define PAGESIZE CONFIG_POSIX_PAGE_SIZE
| ^~~~~~~~~~~~~~~~~~~~~~
ninja: build stopped: subcommand failed.Impact
Annoyance – Minor irritation; no significant impact on usability or functionality.
Environment
- OS: Linux
- Toolchain: Zephyr SDK v0.17.2
- Commit fe5b266
Additional Context
Given that PAGESIZE and PAGE_SIZE are C macros that have appeared in IEEE 1003.1 (i.e. POSIX) for the last 30 years or so, I would say that PAGESIZE is a poor choice for a field name within the USB_Type structure in the NXP HAL in the file PERI_USB.h, since naming a field using the same name as a standard macro is bound to produce undesired results. My guess is that the field names were automatically generated and this case might have slipped by unnoticed during review at NXP.
There may not be a clean way to rename the field, so I'm going to leave it up to the NXP platform people to determine a suitable course of action.