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 uninitialized variables found by address santizer #5304

Merged
merged 3 commits into from Nov 25, 2015

Conversation

Projects
None yet
7 participants
@DeadParrot
Copy link
Contributor

commented Nov 6, 2015

These fixes could cause some diffs -- let's see what the CI testing reveals.

@nrel-bot-2

This comment has been minimized.

Copy link

commented on 7ba1328 Nov 6, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-gcc-4.8: OK (1961 of 1961 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 6, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-cppcheck-1.61: OK (0 of 0 tests passed)

Build Badge

This comment has been minimized.

Copy link

replied Nov 7, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug: OK (887 of 887 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 7, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug: OK (1425 of 1425 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 7, 2015

SanitizerFixes (DeadParrot) - i386-Windows-7-VisualStudio-12: OK (1961 of 1961 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 7, 2015

SanitizerFixes (DeadParrot) - Win64-Windows-7-VisualStudio-12: OK (1961 of 1961 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 8, 2015

SanitizerFixes (DeadParrot) - x86_64-MacOS-10.9-clang: OK (1955 of 1955 tests passed)

Build Badge Test Badge

@nrel-bot

This comment has been minimized.

Copy link

commented Nov 21, 2015

@DeadParrot @lgentile it has been 14 days since this pull request was last updated.

@Myoldmopar

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

It looks like no diffs here. It does affect construction of the HVACNodeConvergLogStruct structure, but it doesn't seem to cause a problem.

I'm inclined to merge this in now. @mjwitte @EnergyArchmage does anything pop out to you with these changes?

@DeadParrot

This comment has been minimized.

Copy link
Contributor Author

commented Nov 24, 2015

@Myoldmopar these are actual bugs: those uninitialized variables were being used with non-zero/false values in at least some builds. If fixing these causes a problem then we have other bugs to find.

@@ -101,6 +101,10 @@ namespace DataConvergParams {

// Default Constructor
HVACNodeConvergLogStruct() :
NodeNum( 0 ),
NotConvergedHumRate( false ),

This comment has been minimized.

Copy link
@mjwitte

mjwitte Nov 24, 2015

Contributor

@DeadParrot These three NotConverge* variables never seem to be used. @EnergyArchmage Can we delete them and the loops that are setting them? Or did I miss something?

This comment has been minimized.

Copy link
@EnergyArchmage

EnergyArchmage Nov 24, 2015

Contributor

Yes I guess they can be deleted. I don't recall the details as to why they are there but I agree they can be deleted.

@DeadParrot

This comment has been minimized.

Copy link
Contributor Author

commented Nov 24, 2015

@mjwitte I'm fine with you or @EnergyArchmage doing the "pruning" on this branch to avoid needing a separate PR. Or confirm that it is NotConvergedHumRate, NotConvergedMassFlow, and NotConvergedTemp in these diffs and I can do it.

@mjwitte

This comment has been minimized.

Copy link
Contributor

commented Nov 24, 2015

@DeadParrot In DataConvergParams.hh HVACNodeConvergLogStruct, you can delete NotConvergedHumRate, NotConvergedMassFlow, and NotConvergedTemp.

They are assigned here in HVACManager::SimHVAC and at various points in the same for loop. Those lines can be removed.

@DeadParrot

This comment has been minimized.

Copy link
Contributor Author

commented Nov 24, 2015

@mjwitte OK, done.

@nrel-bot-3

This comment has been minimized.

Copy link

commented on a7b6bf0 Nov 24, 2015

SanitizerFixes (DeadParrot) - x86_64-MacOS-10.9-clang: Tests Failed

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-gcc-4.8: Tests Failed

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-cppcheck-1.61: OK (0 of 0 tests passed)

Build Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - i386-Windows-7-VisualStudio-12: Tests Failed

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug: OK (887 of 887 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug: OK (1425 of 1425 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - Win64-Windows-7-VisualStudio-12: Tests Failed

Build Badge Test Badge

@nrel-bot-3

This comment has been minimized.

Copy link

commented on 3625086 Nov 24, 2015

SanitizerFixes (DeadParrot) - x86_64-MacOS-10.9-clang: OK (1965 of 1965 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-gcc-4.8: OK (1971 of 1971 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-cppcheck-1.61: OK (0 of 0 tests passed)

Build Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug: OK (897 of 897 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug: OK (1435 of 1435 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - i386-Windows-7-VisualStudio-12: OK (1971 of 1971 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 25, 2015

SanitizerFixes (DeadParrot) - Win64-Windows-7-VisualStudio-12: OK (1971 of 1971 tests passed)

Build Badge Test Badge

@DeadParrot

This comment has been minimized.

Copy link
Contributor Author

commented Nov 25, 2015

I merged in develop but there are still CI fails due to fewer variables. No idea...

@mjwitte

This comment has been minimized.

Copy link
Contributor

commented Nov 25, 2015

Looks all good now.

@mjwitte

This comment has been minimized.

Copy link
Contributor

commented Nov 25, 2015

@Myoldmopar This looks ready to drop in.

mjwitte added a commit that referenced this pull request Nov 25, 2015

Merge pull request #5304 from NREL/SanitizerFixes
Fixes for bugs found by address santizer

@mjwitte mjwitte merged commit 5de24a0 into develop Nov 25, 2015

7 checks passed

Win64-Windows-7-VisualStudio-12 OK (1971 of 1971 tests passed)
Details
i386-Windows-7-VisualStudio-12 OK (1971 of 1971 tests passed)
Details
x86_64-Linux-Ubuntu-14.04-cppcheck-1.61 OK (0 of 0 tests passed)
Details
x86_64-Linux-Ubuntu-14.04-gcc-4.8 OK (1971 of 1971 tests passed)
Details
x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug OK (1435 of 1435 tests passed)
Details
x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug OK (897 of 897 tests passed)
Details
x86_64-MacOS-10.9-clang OK (1965 of 1965 tests passed)
Details

@mjwitte mjwitte deleted the SanitizerFixes branch Nov 25, 2015

@mjwitte mjwitte added the Defect label Nov 25, 2015

@mjwitte mjwitte changed the title Fixes for bugs found by address santizer Fix uninitialized variables found by address santizer Nov 25, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.