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

Fix a few uninitialized variables #305

Merged

Conversation

cpaelzer
Copy link

@cpaelzer cpaelzer commented Mar 30, 2022

Hi,
while collecting some stable fixes for Ubuntu I found one patch that - AFAICS - wasn't upstreamed yet.
But I think these changes make sense to the project in general just as much.
Therefore I've prepared those for your consideration.

I kept Authorship info intact (= @vorlonofportland) but signed-off myself as I reworked them to match your commit styles.

P.S. I haven't found these as issues or open PRs nor in any recent branches. If I just missed them I beg your pardon and feel free to close.

@coveralls
Copy link

coveralls commented Mar 30, 2022

Pull Request Test Coverage Report for Build 2471

  • 2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.01%) to 77.46%

Files with Coverage Reduction New Missed Lines %
src/tpm2/crypto/openssl/Helpers.c 2 90.23%
src/tpm12/tpm_ticks.c 3 91.97%
Totals Coverage Status
Change from base Build 2439: -0.01%
Covered Lines: 29211
Relevant Lines: 37711

💛 - Coveralls

@stefanberger
Copy link
Owner

Thanks for the PR. I would say both of these are false positives and I think we should mark them as such in the patch title. The title of the patches should probably be: Initialize variables due to gcc complaint (s390x, false positive)

@cpaelzer cpaelzer force-pushed the fix-uninitialized-vars-UPSTREAM branch from f9b9006 to f4f9fe5 Compare March 30, 2022 12:08
@cpaelzer
Copy link
Author

Thanks for the PR. I would say both of these are false positives and I think we should mark them as such in the patch title. The title of the patches should probably be: Initialize variables due to gcc complaint (s390x, false positive)

Done and pushed to my proposed branch.

Thanks for the quick review!

@stefanberger stefanberger merged commit 7a64b3e into stefanberger:master Mar 30, 2022
@stefanberger
Copy link
Owner

@cpaelzer Do you still need to apply these 2 patches? I am asking because I don't need to apply them when building for my Ubuntu PPA for Bionic, Focal, and Jammy. So maybe gcc for s390x has been improved in the meantime.

@cpaelzer
Copy link
Author

@cpaelzer Do you still need to apply these 2 patches? I am asking because I don't need to apply them when building for my Ubuntu PPA for Bionic, Focal, and Jammy. So maybe gcc for s390x has been improved in the meantime.

Hi @stefanberger, indeed I haven't checked that before.

If I read the history correctly the bad gcc was at the late days of Impish which is still out there and makes it worth to keep the changes merged either way.

But on your hint I was dropping it from my 0.9.3 merge and rebuilt on all platforms - indeed for Jammy I was able to drop it without consequence.

@stefanberger
Copy link
Owner

The question is whether to backport them to 0.9.x to allow it to build without having to apply these patches. I guess you would want me to apply them to stable-0.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants