Skip to content
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.

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

Remove definitions of legacy symbols #9492

Merged

Conversation

eleuzi01
Copy link
Contributor

@eleuzi01 eleuzi01 commented Aug 21, 2024

Description

Resolves #9396

Removal of symbols from #9385, #9540, and #9544 to be included in their respective PRs.

PR checklist

Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

Help make review efficient:

  • Multiple simple commits
    • please structure your PR into a series of small commits, each of which does one thing
  • Avoid force-push
    • please do not force-push to update your PR - just add new commit(s)
  • See our Guidelines for Contributors for more details about the review process.

@eleuzi01 eleuzi01 added enhancement needs-work component-crypto Crypto primitives and low-level interfaces needs-ci Needs to pass CI tests size-s Estimated task size: small (~2d) priority-high High priority - will be reviewed soon labels Aug 21, 2024
@eleuzi01 eleuzi01 force-pushed the remove-legacy-symbol-definitions branch from 3378c8c to 0f06926 Compare August 21, 2024 10:37
@eleuzi01 eleuzi01 added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review and removed needs-work needs-ci Needs to pass CI tests labels Aug 22, 2024
@eleuzi01 eleuzi01 force-pushed the remove-legacy-symbol-definitions branch from 0bbe87a to f00458e Compare September 3, 2024 15:56
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

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

I've had a first look and I have a few questions.

include/mbedtls/config_adjust_legacy_crypto.h Outdated Show resolved Hide resolved
include/mbedtls/check_config.h Outdated Show resolved Hide resolved
include/mbedtls/check_config.h Outdated Show resolved Hide resolved
Copy link
Contributor

@gabor-mezei-arm gabor-mezei-arm left a comment

Choose a reason for hiding this comment

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

This PR depends on Mbed-TLS/mbedtls-framework#31

include/mbedtls/check_config.h Outdated Show resolved Hide resolved
@gabor-mezei-arm gabor-mezei-arm added the needs-preceding-pr Requires another PR to be merged first label Sep 5, 2024
@eleuzi01 eleuzi01 force-pushed the remove-legacy-symbol-definitions branch from c278c4f to cb33040 Compare September 5, 2024 19:14
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

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

I've done a complete review now.

@eleuzi01 eleuzi01 reopened this Sep 9, 2024
@eleuzi01 eleuzi01 force-pushed the remove-legacy-symbol-definitions branch 2 times, most recently from 01965ef to e95d7eb Compare September 9, 2024 08:27
@eleuzi01 eleuzi01 removed the needs-reviewer This PR needs someone to pick it up for review label Sep 18, 2024
Copy link
Contributor

@gabor-mezei-arm gabor-mezei-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

Remove straightforward definitions that only appear in
config_adjust_legacy_crypto.h

Signed-off-by: Elena Uziunaite <elena.uziunaite@arm.com>
Signed-off-by: Elena Uziunaite <elena.uziunaite@arm.com>
Replace symbols that were left over in config_adjust_*.h and
check_config.h

Signed-off-by: Elena Uziunaite <elena.uziunaite@arm.com>
Signed-off-by: Elena Uziunaite <elena.uziunaite@arm.com>
Signed-off-by: Elena Uziunaite <elena.uziunaite@arm.com>
Signed-off-by: Elena Uziunaite <elena.uziunaite@arm.com>
Signed-off-by: Elena Uziunaite <elena.uziunaite@arm.com>
Signed-off-by: Elena Uziunaite <elena.uziunaite@arm.com>
@eleuzi01 eleuzi01 force-pushed the remove-legacy-symbol-definitions branch from e701ea8 to 66c606a Compare September 26, 2024 13:50
Signed-off-by: Elena Uziunaite <elena.uziunaite@arm.com>
@eleuzi01 eleuzi01 force-pushed the remove-legacy-symbol-definitions branch from 66c606a to bf90fa9 Compare September 26, 2024 14:23
@eleuzi01 eleuzi01 added needs-ci Needs to pass CI tests and removed needs-preceding-pr Requires another PR to be merged first needs-ci Needs to pass CI tests labels Sep 26, 2024
Copy link
Contributor

@ronald-cron-arm ronald-cron-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, thanks.

Copy link
Contributor

@gabor-mezei-arm gabor-mezei-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

ronald-cron-arm added a commit to Mbed-TLS/mbedtls-framework that referenced this pull request Sep 30, 2024
Replace MBEDTLS_PK_HAVE_ECC_KEYS with PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY

Validated for dev by the CI of Mbed-TLS/mbedtls#9492, for 3.6 by the CI of Mbed-TLS/mbedtls#9640.
Signed-off-by: Elena Uziunaite <elena.uziunaite@arm.com>
Copy link
Contributor

@ronald-cron-arm ronald-cron-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

Copy link
Contributor

@gabor-mezei-arm gabor-mezei-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

@gabor-mezei-arm gabor-mezei-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 Oct 1, 2024
@ronald-cron-arm ronald-cron-arm added this pull request to the merge queue Oct 1, 2024
Merged via the queue into Mbed-TLS:development with commit 96c6f5e Oct 1, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports component-crypto Crypto primitives and low-level interfaces enhancement priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)
Development

Successfully merging this pull request may close these issues.

Remove definitions of some legacy symbols
5 participants