drivers: reset: Use of reserved identifier assert

[stephanosio]

Describe the bug

The reset controller driver header file makes use of the identifier assert, which is reserved by the C standard and should not be defined outside the C library:

zephyr/include/zephyr/drivers/reset.h

Line 140 in 25e6279

reset_api_assert assert;

This causes a conflict with the newlib assert.h header, which defines the assert macro that gets expanded when accessing the reset controller driver API struct member assert.

zephyr/include/zephyr/drivers/reset.h

Line 209 in 25e6279

if (api->assert == NULL) {

Logs and console output

/home/stephanos/Dev/zephyrproject/zephyr/include/zephyr/drivers/reset.h: In function 'z_impl_reset_assert':
/home/stephanos/Dev/zephyrproject/zephyr/include/zephyr/drivers/reset.h:213:28: error: macro "assert" passed 2 arguments, but takes just 1
  213 |  return api->assert(dev, id);

In file included from /opt/zephyr-sdk-0.14.2/arm-zephyr-eabi/arm-zephyr-eabi/sys-include/sys/reent.h:503,
                 from /opt/zephyr-sdk-0.14.2/arm-zephyr-eabi/arm-zephyr-eabi/sys-include/sys/errno.h:11,
                 from /opt/zephyr-sdk-0.14.2/arm-zephyr-eabi/arm-zephyr-eabi/sys-include/errno.h:9,
                 from /home/stephanos/Dev/zephyrproject/zephyr/include/zephyr/drivers/uart.h:23,
                 from /home/stephanos/Dev/zephyrproject/zephyr/drivers/serial/uart_rpi_pico.c:8:
/opt/zephyr-sdk-0.14.2/arm-zephyr-eabi/arm-zephyr-eabi/sys-include/assert.h:16: note: macro "assert" defined here
   16 | # define assert(__e) ((__e) ? (void)0 : __assert_func (__FILE__, __LINE__, \
      |

To Reproduce

Include newlib assert.h and drivers/reset.h together.

Impact

Unable to use the drivers/reset.h alongside the C library assert.h.

Environment (please complete the following information):

• OS: Ubuntu 20.04
• Toolchain: Zephyr SDK 0.14.2
• Commit SHA: 25e6279

Additional context

Discovered in #45932

[stephanosio]

FYI @andrei-edward-popa

Could you please look into this?

[andrei-edward-popa]

FYI @andrei-edward-popa

Could you please look into this?

Hi! I understand the issue, but if you include assert.h after drivers/reset.h the code compiles because the assert name is not replaced by the assert define. If it is necessary, I can change assert and deassert into something else. @gmarull What do you think?

[fabiobaltieri]

For context, the problem appears with a reset driver (there's only reset_rpi_pico.c for now) and CONFIG_NEWLIB_LIBC=y. For example west build -p -b rpi_pico samples/basic/blinky -DCONFIG_NEWLIB_LIBC=y fails.

[andrei-edward-popa]

For context, the problem would appears with a reset driver (there's only reset_rpi_pico.c for now) and CONFIG_NEWLIB_LIBC=y. For example west build -p -b rpi_pico samples/basic/blinky -DCONFIG_NEWLIB_LIBC=y fails.

Oh, it is clear for me now. I have 2 options to fix this: to change the name from assert and deassert to something else (I am open to any suggestions for this), or to insert "#undef assert" into include/zephyr/reset.h. @carlocaione What do you think?

[fabiobaltieri]

@stephanosio mentioned on Discord that this API was never part of a release, so it should be ok to modify without going through the API change process. Guess if we want to change it (I say it's the right thing to do) we should do it before v3.1 gets cut.

Not sure about the naming... set/unset?

[stephanosio]

to insert "#undef assert" into include/zephyr/reset.h

That is going to be problematic if there is anything after the inclusion of reset.h that actually makes use of the assert macro from the assert.h.

to change the name from assert and deassert to something else

That would be the most viable solution in my opinion; though, it might be feasible to keep the assert/deassert nomenclature and simply prefix (or suffix) it (e.g. line_assert and same for line_deassert for the sake of consistency).

cc @carlescufi

[carlocaione]

Fine to me the name change and the line_{assert,deassert} proposal

[andrei-edward-popa]

Cool, I will do a PR today with those changes.