Missing include to limits.h in ssl_tls.c

[JonatanAntoni]

I get compilation errors at
https://github.com/ARMmbed/mbedtls/blob/8266acacc8d6e1c65fba9a048f56339d0827b2fe/library/ssl_tls.c#L2446
stating

error: use of undeclared identifier 'INT_MAX'

I think the include to limits.h is missing.

[RonEld]

@JonatanAntoni Thank you for raising this issue!
Could you share your compilation flags and toolchain?

[JonatanAntoni]

Its MDK-ARM using mbedTLS through CMSIS-mbedTLS, running with Arm Compiler 6.10.1.

Compiler flags are:

-xc -std=c99 --target=arm-arm-none-eabi -mcpu=cortex-m33+nodsp
-mfpu=none -mfloat-abi=soft -mcmse -mlittle-endian -gdwarf-3 -O0
-fno-rtti -funsigned-char -fshort-enums -fshort-wchar -ffunction-sections
-c

[ciarmcom]

ARM Internal Ref: IOTSSL-2376

[redtangent]

limits.h is only included in the check_config.h header, so if a user passes their own configuration file and doesn't include check_config.h, then they'll fail to include limits.h. It's also missing from programs/test/udp_proxy.c.

I've provided a fix in PR #1999.

The project could consider a test in all.sh with a custom config.h that omits the check_config.h.

[LaszloLango]

The same issue here with the latest release. :(

[RonEld]

@LaszloLango this issue has been fixed in #1999 , unfortunately, it hasn't been merged yet

[tom-cosgrove-arm]

Closing this as udp_proxy.c now includes build_info.h, and ssl_tls.c includes common.h, which includes build_info.h

build_info.h includes check_config.h which includes <limits.h>

[gilles-peskine-arm]

Is this still relevant for 2.28?

[tom-cosgrove-arm]

On 2.28, if check_config.h is omitted from a user-supplied config file, then yes, the same problem will occur (it's no longer ssl_tls.c now, but ctr_drbg.c, psa_crypto_rsa.c, ssl_msg.c and x509_crt.c with udp_proxy.c, not to mention in the tests).

However, if we think that this is a problem (building with a user-supplied config that doesn't include check_config.h), I think it would be much better to make 2.28 like development with build_info.h - reducing the diff between the two, and fixing potential problems like this at the same time.

Addressing just this issue would make 2.28 and development diverge even more, making our LTS work harder.

We could of course just document (in 2.28) that any user-supplied config needs to include check_config.h.

Whatever we do, "Missing include to limits.h in ssl_tls.c" isn't correct any more.

[gilles-peskine-arm]

For ease of maintainability, source files should explicitly include headers if they use definitions from them. That's an issue even in 3.x.

I remember limits.h coming up before in pull requests. But it might have been tangled in other changes that didn't get merged.

We recommend including check_config.h in user configurations, but it isn't mandatory, and builds should pass without it. (That's in 2.x: in 3.x the inclusion of check_config.h has moved out of the user-editable file and is now automatic.)