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

GetGuid noexcept Fix #3806

Merged
12 commits merged into from
Dec 3, 2019
19 changes: 9 additions & 10 deletions src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1280,23 +1280,25 @@ namespace TerminalAppLocalTests
"guid": "{6239a42c-6666-49a3-80bd-e8fdd045185c}"
},
{
"name" : "Ubuntu",
"guid" : "{2C4DE342-38B7-51CF-B940-2309A097F518}"
"name" : "ThisProfileShouldNotThrow"
},
{
"name" : "ThisProfileShouldNotCrash"
"name" : "Ubuntu",
"guid" : "{2C4DE342-38B7-51CF-B940-2309A097F518}"
}
]
})" };

auto name0{ L"profile0" };
auto name1{ L"profile1" };
auto name2{ L"Ubuntu" };
auto name3{ L"ThisProfileShouldNotThrow" };
auto badName{ L"DoesNotExist" };

auto guid0{ Microsoft::Console::Utils::GuidFromString(L"{6239a42c-5555-49a3-80bd-e8fdd045185c}") };
auto guid1{ Microsoft::Console::Utils::GuidFromString(L"{6239a42c-6666-49a3-80bd-e8fdd045185c}") };
auto guid2{ Microsoft::Console::Utils::GuidFromString(L"{2C4DE342-38B7-51CF-B940-2309A097F518}") };
auto fakeGuid{ Microsoft::Console::Utils::GuidFromString(L"{FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF}") };
std::optional<GUID> badGuid{};

VerifyParseSucceeded(settings0String);
Expand All @@ -1308,22 +1310,19 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(guid0, settings.FindGuid(name0));
VERIFY_ARE_EQUAL(guid1, settings.FindGuid(name1));
VERIFY_ARE_EQUAL(guid2, settings.FindGuid(name2));
VERIFY_ARE_EQUAL(badGuid, settings.FindGuid(name3));
VERIFY_ARE_EQUAL(badGuid, settings.FindGuid(badName));
// Following test will fail because GetGuid throws (despite being noexcept)
// VERIFY_ARE_EQUAL(badGuid, settings.FindGuid(L"ThisProfileShouldNotCrash"));

// Following lines only work because the GUID-less profile is last
auto prof0{ settings.FindProfile(guid0) };
auto prof1{ settings.FindProfile(guid1) };
auto prof2{ settings.FindProfile(guid2) };
// Following line will fail because GetGuid throws (despite being noexcept)
// auto badProf{ settings.FindProfile(badGuid) };

auto badProf{ settings.FindProfile(fakeGuid) };
VERIFY_ARE_EQUAL(badProf, nullptr);

VERIFY_ARE_EQUAL(name0, prof0->GetName());
VERIFY_ARE_EQUAL(name1, prof1->GetName());
VERIFY_ARE_EQUAL(name2, prof2->GetName());
// Following test will fail because GetGuid throws (despite being noexcept)
// VERIFY_ARE_EQUAL(badName, badProf->GetName());
}

void SettingsTests::TestLayerGlobalsOnRoot()
Expand Down
15 changes: 12 additions & 3 deletions src/cascadia/TerminalApp/CascadiaSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,12 @@ std::optional<GUID> CascadiaSettings::FindGuid(const std::wstring& profileName)
{
if (profileName == profile.GetName())
{
profileGuid = profile.GetGuid();
try
{
profileGuid = profile.GetGuid();
}
CATCH_LOG();

break;
}
}
Expand All @@ -98,10 +103,14 @@ const Profile* CascadiaSettings::FindProfile(GUID profileGuid) const noexcept
{
for (auto& profile : _profiles)
{
if (profile.GetGuid() == profileGuid)
try
{
return &profile;
if (profile.GetGuid() == profileGuid)
{
return &profile;
}
}
CATCH_LOG();
}
return nullptr;
}
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/Profile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ bool Profile::HasSource() const noexcept
return _source.has_value();
}

GUID Profile::GetGuid() const noexcept
GUID Profile::GetGuid() const
{
// This can throw if we never had our guid set to a legitimate value.
THROW_HR_IF_MSG(E_FAIL, !_guid.has_value(), "Profile._guid always expected to have a value");
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/Profile.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class TerminalApp::Profile final

bool HasGuid() const noexcept;
bool HasSource() const noexcept;
GUID GetGuid() const noexcept;
GUID GetGuid() const;
void SetSource(std::wstring_view sourceNamespace) noexcept;
std::wstring_view GetName() const noexcept;
bool HasConnectionType() const noexcept;
Expand Down