mbedtls icon indicating copy to clipboard operation
mbedtls copied to clipboard

ssl_tls13_generic.c(228) variable "ret" was set but never used

Open VitSolcikNXP opened this issue 3 months ago • 3 comments

https://github.com/Mbed-TLS/mbedtls/blob/d4b6bc74e92b1411648fbbe4ea1f1e3595392cb4/library/ssl_tls13_generic.c#L219

This return variable is used only in MBEDTLS_SSL_DEBUG_RET macro. When it is not used, compilator throws warning: library/ssl_tls13_generic.c(228) : Warning[Pe550]: variable "ret" was set but never used

Can this be re-worked? Maybe just used void typecast: (void) ret;

Thank you!

VitSolcikNXP avatar Oct 13 '25 12:10 VitSolcikNXP

This looks like a relatively simple fix - as you point out, we should be able to add (void) ret; to solve the issue.

Steps to complete:

  • Build without MBEDTLS_DEBUG_C to reproduce the warning
  • ~Add the (void) ret; line~ Use the zero value of ret when we call mbedtls_pk_verify_new() as discussed below.
  • Check that the warning goes away

Already labelled: size-xs.

davidhorstmann-arm avatar Oct 16 '25 13:10 davidhorstmann-arm

This looks like a relatively simple fix - as you point out, we should be able to add (void) ret; to solve the issue.

Such fix would be a waste of resources, would it not? The fix suppresses the warning instead of eliminating the real issues, also increases the size of source code. The issues are: declared variable is assigned an unusable value, and returning constant zero instead of zero value received from the function call.

While the library explicitly requires C99, a lot of recently written code still uses idioms from K&R days (if not older). In this case, the code could be:

int ret = mbedtls_pk_verify_new(sig_alg, ...);
if (ret == 0) {
    return ret;
}

And return ret; should be better than (void)ret; in any case.

irwir avatar Oct 18 '25 13:10 irwir

Ah I suppose you're right that using the zero value from ret would be a neater fix, thanks!

Even so, I think the crux of the issue is that we care about the actual value of ret when we have debug enabled (we want to print its value for debugging) whereas we don't care much about the value of ret when we're not debugging. We only care about whether it was zero (in which case, return 0) or failure (in which case, return MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE so we can terminate the handshake).

davidhorstmann-arm avatar Oct 20 '25 09:10 davidhorstmann-arm