Fix uninitialized variable in x509_crt (#2392 resubmit)

[pfalcon]

This patch fixes an issue we encountered with more stringent compiler
warnings. The signature_is_good variable has a possibility of being
used uninitialized. This patch moves the use of the variable to a
place where it cannot be used while uninitialized.

Signed-off-by: Andy Gross andy.gross@linaro.org

Backport status: this has been backported to 2.16 in #2813 and is not applicable to 2.7.

Notes:

• Pull requests cannot be accepted until:

• The submitter has accepted the online agreement here with a click through
  or for companies or those that do not wish to create an mbed account, a slightly different agreement can be found here
• The PR follows the mbed TLS coding standards

• This is just a template, so feel free to use/remove the unnecessary things

Description

A few sentences describing the overall goals of the pull request's commits.

Status

READY/IN DEVELOPMENT/HOLD

Requires Backporting

When there is a bug fix, it should be backported to all maintained and supported branches.
Changes do not have to be backported if:

• This PR is a new feature\enhancement
• This PR contains changes in the API. If this is true, and there is a need for the fix to be backported, the fix should be handled differently in the legacy branch

Yes | NO
Which branch?

Migrations

If there is any API change, what's the incentive and logic for it.

YES | NO

Additional comments

Any additional information that could be of interest

Todos

• Tests
• Documentation
• Changelog updated
• Backported

Steps to test or reproduce

Outline the steps to test or reproduce the PR here.

[pfalcon]

This is resubmit of #2392, implementing following change request there (#2392 (review)):

Could you remove the double space, and perhaps say something like "Improve code clarity in X.509 module, removing false-positive uninitialized variable warnings on some toolchains."? "Fix compiler warning" is a bit too generic, and it doesn't entail whether the warning was justified and potentially hinting at a serious bug.

Comparing to the original patch, only changes to the ChangeLog file content was made. Everything else: the actual code patch, commit message, sign-off, author - stays the same.

@hanno-arm, @mpg, @RonEld (reviewers of #2392), could you please have a look?

[mpg]

I checked that the code changes are identical to those in #2392 which I previously approved.

[k-stachowiak]

I went through discussion in the original PR and through the changes. The code looks good.

[mpg]

@pfalcon Thanks for opening this new PR. Could you check if the warning also happens in our LTS branches (currently mbedtls-2.16 and mbedtls-2.7)? If it does, we'll need PRs backporting the fixes to those branches as well. Thanks!

[pfalcon]

@mpg, @k-stachowiak: Thanks for the review!

Could you check if the warning also happens in our LTS branches (currently mbedtls-2.16 and mbedtls-2.7)?

This does happen in 2.16, that's actually which version it was originally prepared for. I submitted #2813 for that, the only change is moving ChangeLog entry to the appropriate section.

While doing so, it occurred to me that this PR contained ChangeLog entry in an old section (circa 2.16.0), so I moved to the next-version section here (while rebasing on the latest development). That dismissed your reviews, sorry about that, hopefully we can recover them soon ;-).

This patch doesn't apply to 2.7 - the code considerably different there.

[mpg]

Thanks for updating! Still looks good to me.

[mpg]

This has two approving review, passes CI, and so does the only relevant backport, so labeling "ready for merge".