Skip to content

hal: nxp: mcux: usb_type struct in peri_usb.h defines pagesize field #95285

@cfriedt

Description

@cfriedt

Describe the bug

The weekly CI run shows an summary of an error below, with 270 hits.

error: expected identifier or '(' before numeric constant

The 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

  1. west build -p auto -b verdin_imx8mp/mimx8ml8/m7 tests/lib/c_lib/common -T libraries.libc.common.minimal
  2. 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.

Metadata

Metadata

Labels

bugThe issue is a bug, or the PR is fixing a bugplatform: NXPNXPpriority: mediumMedium impact/importance bug

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions