Fix "inconsistent annotation" warnings

[irwir]

Suggested enhancement

All functions declared as MBEDTLS_CHECK_RETURN_CRITICAL should have this annotation in definitions.

Justification

Currently some functions are declared in ssl_misc.h and defined in ssl_tls.c with MBEDTLS_CHECK_RETURN_CRITICAL annotation. For unknown reason there are definitions with omitted annotation, and this gives warnings in VS 2022. The warning is minor, but for consistency and quiet compilation this should be amended.

[tom-daubney-arm]

Hi, thanks for the report.

Some specific examples of the functions you describe would be really helpful. I would invite you to raise a PR against the respository which implements the fix you want to see. This would be the fastest way to close the issue. Would you have time to do this? In the meantime I will have a look and try to replicate the isue, although I am not a Windows user. Thanks

[irwir]

int mbedtls_ssl_session_copy(mbedtls_ssl_session *dst, int mbedtls_ssl_add_hs_hdr_to_checksum(mbedtls_ssl_context *ssl, int mbedtls_ssl_add_hs_hdr_to_checksum(mbedtls_ssl_context *ssl, int mbedtls_ssl_add_hs_msg_to_checksum(mbedtls_ssl_context *ssl, int mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl) int mbedtls_ssl_session_reset_int(mbedtls_ssl_context *ssl, int partial) int mbedtls_ssl_conf_has_static_psk(mbedtls_ssl_config const *conf) int mbedtls_ssl_start_renegotiation(mbedtls_ssl_context *ssl) int mbedtls_ssl_check_curve_tls_id(const mbedtls_ssl_context *ssl, uint16_t tls_id) int mbedtls_ssl_check_curve(const mbedtls_ssl_context *ssl, mbedtls_ecp_group_id grp_id) int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, int mbedtls_ssl_get_handshake_transcript(mbedtls_ssl_context *ssl, int mbedtls_ssl_parse_sig_alg_ext(mbedtls_ssl_context *ssl, int mbedtls_ssl_derive_keys(mbedtls_ssl_context *ssl) int mbedtls_ssl_set_calc_verify_md(mbedtls_ssl_context *ssl, int md) int mbedtls_ssl_psk_derive_premaster(mbedtls_ssl_context *ssl, mbedtls_key_exchange_type_t key_ex) int mbedtls_ssl_write_certificate(mbedtls_ssl_context *ssl) int mbedtls_ssl_parse_certificate(mbedtls_ssl_context *ssl) int mbedtls_ssl_write_finished(mbedtls_ssl_context *ssl) int mbedtls_ssl_parse_finished(mbedtls_ssl_context *ssl) int mbedtls_ssl_get_key_exchange_md_tls1_2(mbedtls_ssl_context *ssl, int mbedtls_ssl_validate_ciphersuite( int mbedtls_ssl_write_sig_alg_ext(mbedtls_ssl_context *ssl, unsigned char *buf, int mbedtls_ssl_write_alpn_ext(mbedtls_ssl_context *ssl,

Hope it helps.

[tom-daubney-arm]

For further information, are there any specific build options you use when these warnings appear or is it a case of any and all builds will have these warnings?

[irwir]

Probably it was this one: https://learn.microsoft.com/en-us/cpp/code-quality/c28252?view=msvc-170 Then code analysis should be enabled.

[tom-daubney-arm]

Sorry I mean are you passing any specific build options etc to the compiler that trigger the warnings? Or are you jsut building with everything default and seeing them?

e: I didn't realise you had to manually enable some warnings in VS, now I get why you sent the link. Thanks for that.

[tom-daubney-arm]

Just to update: We have tried to replicate these warnings on VS 2022 but have as yet been unable to do so, so we are going to need more information about the steps needed to reproduce the issue.

Please may we have your full reproduction steps, including things like retarget options, system evironment etc. The mbedtls version you are using would be good too. The more you can tell us the better. Thanks

[tom-daubney-arm]

We have now verified the existence of these warnings. Fixing this would be an improvement but it is not something we can prioritise at present. We would welcome a PR from the community for this. Also, it should be noted that the full fix for this would include having something in the CI to recreate the VS build that shows these warnings.

[irwir]

There was an odd and failed attempted fix; the issue survived and got bigger. Now there are many more functions with non-annotated definitions. Additional list of files might be incomplete, but at least these: ssl_client.c ssl_tls13_generic.c ssl_tls13_keys.c And in TF-PSA-crypto: aes.c

Have not checked annottation MBEDTLS_CHECK_RETURN_TYPICAL, it could create similar issues.