Skip to content

[Backport 3.6] Move the inclusion of crypto_sizes.h and crypto_struct.h in crypto.h #10196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 28, 2025
Merged

[Backport 3.6] Move the inclusion of crypto_sizes.h and crypto_struct.h in crypto.h #10196

merged 2 commits into from
May 28, 2025

Conversation

@ronald-cron-arm
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm commented May 28, 2025 •
edited
Loading

Description

Backport of Mbed-TLS/TF-PSA-Crypto#260

PR checklist

@ronald-cron-arm
Move the inclusion of crypto_sizes.h and crypto_struct.h in crypto.h
3ea3635 
That way when API are declared, the types they use are defined.

This should resolve the issues related to psa_xyz_init functions
returning a structure described in Mbed-TLS#7087.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
@ronald-cron-arm
Add change log
4960825 
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
@ronald-cron-arm ronald-cron-arm mentioned this pull request May 28, 2025
Merged
4 tasks
@ronald-cron-arm ronald-cron-arm added needs-ci Needs to pass CI tests priority-medium Medium priority - this can be reviewed as time permits size-xs Estimated task size: extra small (a few hours at most) labels May 28, 2025
gilles-peskine-arm
gilles-peskine-arm approved these changes May 28, 2025
View reviewed changes
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gilles-peskine-arm gilles-peskine-arm added the needs-review Every commit must be reviewed by at least two team members, label May 28, 2025
@ronald-cron-arm ronald-cron-arm removed the needs-ci Needs to pass CI tests label May 28, 2025
davidhorstmann-arm
davidhorstmann-arm approved these changes May 28, 2025
View reviewed changes
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, faithful backport!

@github-project-automation github-project-automation bot moved this from In Development to Has Approval in Non-roadmap pull requests May 28, 2025
@davidhorstmann-arm davidhorstmann-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels May 28, 2025
@davidhorstmann-arm davidhorstmann-arm added this pull request to the merge queue May 28, 2025
Merged via the queue into Mbed-TLS:mbedtls-3.6 with commit 23a0d48 May 28, 2025
6 checks passed
@github-project-automation github-project-automation bot moved this from Has Approval to Done in Non-roadmap pull requests May 28, 2025
@aslze
Copy link

aslze commented May 28, 2025

Hey. Maybe I'm doing something wrong but this is not fixing Bug #7087 in my case. This only changes 'crypto.h' but when I tried I saw two changed files: 'crypto.h' and 'crypto_extras.h'. Both are needed to fix the MSVC build issue. But now I can't find where that 'crypto_extras.h' file was changed; it is not in that PR.

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented May 28, 2025

Ah, I think that other changes to crypto_extra.h in TF-PSA-Crypto resolved the problem. But in the 3.6 LTS branch, I guess we do still need to change it. Thanks for trying it out and reporting back!

@aslze
Copy link

aslze commented May 28, 2025

OK, I see that file was moved to PR258. It seems that PR is meant for another branch (development?) and does not fully work on 3.6.

PR258 changes 2 files:

  • crypto_extra.h is OK and fixes the MSVC issue.
  • psa_crypto_slot_management.c adds an error (a function not found).

So just one changed file from there would be needed.

@ronald-cron-arm ronald-cron-arm mentioned this pull request Jun 3, 2025
Merged
5 tasks
@ronald-cron-arm ronald-cron-arm mentioned this pull request Jun 12, 2025
Merged
4 tasks
@ronald-cron-arm ronald-cron-arm mentioned this pull request Jun 23, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gilles-peskine-arm gilles-peskine-arm gilles-peskine-arm approved these changes

@davidhorstmann-arm davidhorstmann-arm davidhorstmann-arm approved these changes

Assignees

No one assigned

Labels

approved Design and code approved - may be waiting for CI or backports priority-medium Medium priority - this can be reviewed as time permits size-xs Estimated task size: extra small (a few hours at most)

Projects

Non-roadmap pull requests
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ronald-cron-arm @aslze @gilles-peskine-arm @davidhorstmann-arm