Wrong macros in mps_common.h

[Laserdance100]

In mbedtls/library/mps_common.h I have GCC warnings for those:

typedef size_t mbedtls_mps_stored_size_t;
#define MBEDTLS_MPS_STORED_SIZE_MAX ( (mbedtls_mps_stored_size_t) -1 )
typedef size_t mbedtls_mps_size_t;
#define MBEDTLS_MPS_SIZE_MAX ( (mbedtls_mps_size_t) -1 )

It looks like it should be either sizeof(type)-1 or 2^sizeof(type)-1.
Current behavior of GCC is defaulting types values with 0 in the macros.

[paul-elliott-arm]

Hi!

Many thanks for reporting this - could you possibly supply the warnings you are getting, the GCC version and the platform you are targetting?

Many thanks,

Paul.

[gilles-peskine-arm]

The definitions have correct values. But they can't be used in preprocessor conditionals, since the preprocessor doesn't know about types (whether it's through casts, sizeof or whatever). Either we need to change the preprocessor conditionals or to change the definitions to use SIZE_MAX.

[Laserdance100]

Hullo.

I'm using arm-none-eabi-gcc-10.2.1 and my target is STM32F7.
The warning message is as follow: warning: <type_name> is not defined, evaluates to 0, in expansion of macro <*_SIZE_MAX>

As we can read in the doxygen comment "For now, we use the default type of size_t throughout, and the use of smaller types or different types for ::mbedtls_mps_size_t and ::mbedtls_mps_stored_size_t is not yet supported.", I suggest to use as hinted by Gilles:
#define MBEDTLS_MPS_STORED_SIZE_MAX (SIZE_MAX)
#define MBEDTLS_MPS_SIZE_MAX(SIZE_MAX)

This would have to be corrected when the support for different size is added, which is probably far in the future.

One way to go would be as follow:
#if "condition"
typedef uint32_t mbedtls_mps_stored_size_t;
#define MBEDTLS_MPS_STORED_SIZE_MAX "see limit.h"
#else
typedef uint16_t mbedtls_mps_stored_size_t;
#define MBEDTLS_MPS_STORED_SIZE_MAX "see limit.h"
#endif
Which keep the type definition and the type size constant coherent.

Best regards,

Simon

[paul-elliott-arm]

Hi!

I would invite you to make a PR for this here - that would likely be the quickest resolution here, given its a very small PR.

Regards,

Paul.

[tom-cosgrove-arm]

Actually, this would be a good use-case for a putative MBEDTLS_STATIC_ASSERT() - then we could keep the #defines the way they are (which is reasonable) (the issue is using them in #if, as the preprocessor does not know about the new types - switching to a static-type assert would have the compiler handle this)