Ensure that only one definition of ASSERT is present

[Ouss4]

Description

If a previous definition of ASSERT has sneaked in through included files an "ASSERT redefined" warnings would show up.

Status

READY

Steps to test or reproduce

Build with MBEDTLS_SELF_TEST enabled.

[gilles-peskine-arm]

This patch is harmless except for the small maintenance burden, so we'll probably accept it if you really need it. But the fact that you need this patch indicates that something is wrong in your platform headers. What platform is this? How is ASSERT getting defined?

Having one identifier #undef'ed is ok, but we wouldn't want to have #undef for every single identifier that a platform header might possibly define when it shouldn't.

[gilles-peskine-arm]

According to the C standard, #undef ASSERT is enough: it simply has no effect if ASSERT isn't a defined macro. Are there compilers that warn about #undefining a macro that isn't defined?

[Ouss4]

Correct, #undef only is enough.

[gilles-peskine-arm]

A conforming C implementation isn't allowed to define ASSERT: it's in the application namespace.

In this C file, we only include a few standard library headers, some headers of our own, config.h, and possibly platform_alt.h. Where is ASSERT coming from on your build? None of them should define ASSERT, but I realize that some embedded platforms define macros with “convenient” names that are officially reserved for applications.

[Ouss4]

I get it from other libraries I have with the project. I understand that these macros are only used for testing and maybe you don't want to #undef every macro.
It's just common to #undef common macros before defining them to avoid warnings. I'm okay with it if we drop this patch.

[gilles-peskine-arm]

Please add a changelog entry file in ChangeLog.d, something like

Changes
   * Undefine the ASSERT macro before defining it locally, in case it is defined
     in a platform header. Contributed by Abdelatif Guettouche in #3557.

[Ouss4]

Please add a changelog entry file in ChangeLog.d, something like

Changes
   * Undefine the ASSERT macro before defining it locally, in case it is defined
     in a platform header. Contributed by Abdelatif Guettouche in #3557.

Thanks for the text, I added an entry.

[hanno-becker]

LGTM

[mpg]

@gilles-peskine-arm @hanno-arm I just noticed that the file with the ChangeLog entry got created in the wrong directory, so I made a trivial PR to fix this: #3584