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

Silence gcc 12.2.0 warning #9139

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

bluerise
Copy link

@bluerise bluerise commented May 15, 2024

Unfortunately this compiler complains about a variable potentially being used un-initialized. Silence the warning by initializing it to a sane default.

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

Unfortunately this compiler complains about a variable potentially being
used un-initialized.  Silence the warning by initializing it to a sane
default.

Signed-off-by: Patrick Wildt <[email protected]>
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 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 size-xs Estimated task size: extra small (a few hours at most) priority-medium Medium priority - this can be reviewed as time permits needs-backports Backports are missing or are pending review and approval. component-crypto Crypto primitives and low-level interfaces labels May 16, 2024
@@ -263,7 +263,7 @@ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_psa(
{
mbedtls_cipher_mode_t mode;
psa_status_t status;
mbedtls_cipher_id_t cipher_id_tmp;
mbedtls_cipher_id_t cipher_id_tmp = MBEDTLS_CIPHER_ID_NONE;
Copy link
Contributor

Choose a reason for hiding this comment

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

I and our CI can't reproduce the warning with gcc-12 -Wall -Wextra (which includes -Wmaybe-uninitialized). How did you get it? But explicitly initializing a variable isn't a bad thing anyway.

This code also exists in the mbedtls-3.6 and mbedtls-2.28 branches, so please make backport pull requests to these branches. By the way, please keep the “PR checklist” section in the pull request template, we need it for gatekeeping.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't reproduce this with gcc 12.0.1, so I presume it's new with gcc 12.2

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm May 16, 2024

Choose a reason for hiding this comment

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

On my machine and the CI, it's gcc-12 (Ubuntu 12.3.0-1ubuntu1~22.04) 12.3.0 amd64

Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-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. Agree we need backports

@tom-cosgrove-arm tom-cosgrove-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, needs-reviewer This PR needs someone to pick it up for review labels May 16, 2024
@gilles-peskine-arm gilles-peskine-arm added this pull request to the merge queue Jul 4, 2024
Merged via the queue into Mbed-TLS:development with commit e290234 Jul 4, 2024
4 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 needs-backports Backports are missing or are pending review and approval. priority-medium Medium priority - this can be reviewed as time permits size-xs Estimated task size: extra small (a few hours at most)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants