Don't call mbedtls_test_hook_error_add() #254

Merged

gilles-peskine-arm merged 3 commits from error-codes-add-no-hook into development

2025-04-15 13:08:09 -05:00

gilles-peskine-arm commented

2025-04-10 10:49:38 -05:00

(Migrated from github.com)

Fix an omission in https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/167 which is likely to cause test failures when we change more functions to return PSA error codes.

PR checklist

changelog provided | not required because: test stuff
framework PR not required
mbedtls development PR not required because: will be automatically consumed by mbedtls, needs no further work there
mbedtls 3.6 PR not required because: new stuff
tests provided

Fix an omission in https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/167 which is likely to cause test failures when we change more functions to return PSA error codes. ## PR checklist - [x] **changelog** provided | not required because: test stuff - [x] **framework PR** not required - [x] **mbedtls development PR** not required because: will be automatically consumed by mbedtls, needs no further work there - [x] **mbedtls 3.6 PR** not required because: new stuff - **tests** provided

mpg (Migrated from github.com) reviewed

2025-04-11 02:29:06 -05:00

mpg (Migrated from github.com) left a comment

No description provided.

LGTM, thanks! This is resolving the most pressing issue, but not cleaning everything up. I think at some point we should remove all traces of `mbedtls_test_hook_error_add` and `mbedtls_test_err_add_check()` from tf-psa-crypto and mbedtls/development, and guard all occurrences in mbedtls-framework with "If mbedtls version < 4". Doesn't have to be now, but then we probably want an issue to track it?

gilles-peskine-arm commented

2025-04-11 02:37:05 -05:00

(Migrated from github.com)

I think at some point we should remove all traces of mbedtls_test_hook_error_add and mbedtls_test_err_add_check() from tf-psa-crypto

Isn't that what I did? Oh, I forgot to remove the declaration. Updated.

and mbedtls/development

I thought there'd be no impact, but there's a now-useless variable psasim, I'll make a follow-up for that.

and guard all occurrences in mbedtls-framework with "If mbedtls version < 4".

Why? I'd prefer not to have explicit version dependencies if it's not necessary. That will disrupt existing branches, for example. The framework just offers the hook, and it's harmless if a branch doesn't consume that hook.

> I think at some point we should remove all traces of mbedtls_test_hook_error_add and mbedtls_test_err_add_check() from tf-psa-crypto Isn't that what I did? Oh, I forgot to remove the declaration. Updated. > and mbedtls/development I thought there'd be no impact, but there's a now-useless variable psasim, I'll make a follow-up for that. > and guard all occurrences in mbedtls-framework with "If mbedtls version < 4". Why? I'd prefer not to have explicit version dependencies if it's not necessary. That will disrupt existing branches, for example. The framework just offers the hook, and it's harmless if a branch doesn't consume that hook.

mpg (Migrated from github.com) approved these changes

2025-04-11 02:46:29 -05:00

mpg (Migrated from github.com) left a comment

LGTM

mpg commented

2025-04-11 02:48:40 -05:00

(Migrated from github.com)

and guard all occurrences in mbedtls-framework with "If mbedtls version < 4".

Why? I'd prefer not to have explicit version dependencies if it's not necessary. That will disrupt existing branches, for example. The framework just offers the hook, and it's harmless if a branch doesn't consume that hook.

You're right, let's not add a guard if we don't need to.

> > and guard all occurrences in mbedtls-framework with "If mbedtls version < 4". > > Why? I'd prefer not to have explicit version dependencies if it's not necessary. That will disrupt existing branches, for example. The framework just offers the hook, and it's harmless if a branch doesn't consume that hook. You're right, let's not add a guard if we don't need to.

mpg (Migrated from github.com) requested changes

2025-04-11 02:51:23 -05:00

drivers/builtin/include/mbedtls/error_common.h