计算Windows计算器中的错误

几天前，微软公开了他们的Windows计算器的源代码。Calculator是一个传统上随每个Windows版本一起提供的应用程序。近年来，许多微软项目开源，但这次新闻甚至在第一天就被非IT媒体 道。嗯，这是一个流行但很小的C ++程序。尽管它的大小，我们仍然设法使用PVS-Studio静态分析仪在其代码中找到许多可疑片段。

介绍

我认为我们不需要引入计算器，因为您很难找到不知道它是什么的Windows用户。现在任何人都可以从GitHub下载应用程序的源代码并提出改进建议。

例如，以下功能已引起社区的注意：

void TraceLogger::LogInvalidInputPasted(....)
{
 if (!GetTraceLoggingProviderEnabled()) return;
 LoggingFields fields{};
 fields.AddString(L"Mode", NavCategory::GetFriendlyName(mode)->Data());
 fields.AddString(L"Reason", reason);
 fields.AddString(L"PastedExpression", pastedExpression);
 fields.AddString(L"ProgrammerNumberBase", GetProgrammerType(...).c_str());
 fields.AddString(L"BitLengthType", GetProgrammerType(bitLengthType).c_str());
 LogTelemetryEvent(EVENT_NAME_INVALID_INPUT_PASTED, fields);
}

此功能从剪贴板中记录文本，并显然将其发送到Microsoft服务器

。但是，这篇文章并不是关于这个功能，但你肯定会看到很多可疑的片段。

我们使用PVS-Studio静态分析器来检查Calculator的源代码。由于它不是用标准C ++编写的，我们的许多普通读者都怀疑这样的检查是可能的，但我们做到了。该分析仪支持C ++ / CLI和C ++ / CX，即使某些诊断确实产生了一些误 ，我们也没有遇到任何会妨碍PVS-Studio工作的关键问题。

同样提醒一下，如果您错过了有关我们工具其他功能的新闻，PVS-Studio不仅支持C和C ++，还支持C＃和Java。

字符串比较不正确

V547表达式’m_resolvedName == L?en-US?’始终为false。要比较字符串，您应该使用wcscmp（）函数。Calculator LocalizationSettings.h 180

wchar_t m_resolvedName[LOCALE_NAME_MAX_LENGTH];
Platform::String^ GetEnglishValueFromLocalizedDigits(....) const
{
 if (m_resolvedName == L"en-US")
 {
 return ref new Platform::String(localizedString.c_str());
 }
 ....
}

在查看分析器 告时，我按诊断代码按升序对警告进行排序，这个例子就是一个很生动的例子，恰好是列表中的第一个。

你看，上面的例子显示了不正确的字符串比较。事实上程序员通过比较字符数组的地址和字符串文字的地址来比较指针而不是字符串值。这些指针永远不会相等，所以条件也总是假的。为了正确比较字符串，应该使用函数wcscmp。

顺便说一下，在我写这篇文章时，字符数组m_resolvedName在头文件中修复，并成为std :: wstring类型的完整字符串，所以现在可以正确地进行比较。到目前为止你将阅读这篇文章，由于这样的爱好者和评论，许多其他错误也可能会被修复。

本机代码中的内存泄漏

V773退出该函数时未释放’temp’指针。内存泄漏是可能的。CalcViewModel
StandardCalculatorViewModel.cpp 529

void StandardCalculatorViewModel::HandleUpdatedOperandData(Command cmdenum)
{
 ....
 wchar_t* temp = new wchar_t[100];
 ....
 if (commandIndex == 0)
 {
 delete [] temp;
 return;
 }
 ....
 length = m_selectedExpressionLastData->Length() + 1;
 if (length > 50)
 {
 return;
 }
 ....
 String^ updatedData = ref new String(temp);
 UpdateOperand(m_tokenPosition, updatedData);
 displayExpressionToken->Token = updatedData;
 IsOperandUpdatedUsingViewModel = true;
 displayExpressionToken->CommandIndex = commandIndex;
}

所述临时指针是指100种元素的动态分配的数组。不幸的是，内存仅在函数的一部分中释放，而其余部分都以内存泄漏结束。这不是太糟糕，但它仍然被认为是C ++代码中的一个错误。

难以捉摸的例外

V702类应始终从std :: exception（和类似的）派生为’public’（未指定关键字，因此编译器将其默认为’private’）。CalcManager CalcException.h 4

class CalcException : std::exception
{
public:
 CalcException(HRESULT hr)
 {
 m_hr = hr;
 }
 HRESULT GetException()
 {
 return m_hr;
 }
private:
 HRESULT m_hr;
};

分析器使用private修饰符检测到从std :: exception类派生的类（如果未指定其他修饰符，则为默认值）。此代码的问题在于，当尝试捕获泛型std :: exception时，处理程序将忽略CalcException类型的异常，因为私有继承禁止隐式类型转换。

错过了一天

V719 switch语句未涵盖’DateUnit’枚举的所有值：Day。CalcViewModel DateCalculator.cpp 279

public enum class _Enum_is_bitflag_ DateUnit
{
 Year = 0x01,
 Month = 0x02,
 Week = 0x04,
 Day = 0x08
};
Windows::Globalization::Calendar^ m_calendar;
DateTime
DateCalculationEngine::AdjustCalendarDate(Windows::Foundation::DateTime date,
 DateUnit dateUnit, int difference)
{
 m_calendar→SetDateTime(date);
 switch (dateUnit)
 {
 case DateUnit::Year:
 {
 ....
 m_calendar->AddYears(difference);
 m_calendar->ChangeCalendarSystem(currentCalendarSystem);
 break;
 }
 case DateUnit::Month:
 m_calendar->AddMonths(difference);
 break;
 case DateUnit::Week:
 m_calendar->AddWeeks(difference);
 break;
 }
 return m_calendar->GetDateTime();
}

怀疑switch语句没有DateUnit :: Day案例。因此，虽然日历确实具有AddDays方法，但日期值不会添加到日历（m_calendar变量）中。 其他可疑案例与另一个枚举：

实数的可疑比较

V550奇怪的精确比较：比率==阈值。使用具有定义精度的比较可能更好：fabs（A – B）<Epsilon。计算器AspectRatioTrigger.cpp 80

void AspectRatioTrigger::UpdateIsActive(Size sourceSize)
{
 double numerator, denominator;
 ....
 bool isActive = false;
 if (denominator > 0)
 {
 double ratio = numerator / denominator;
 double threshold = abs(Threshold);
 isActive = ((ratio > threshold) || (ActiveIfEqual && (ratio == threshold)));
 }
 SetActive(isActive);
}

分析仪指出可疑表达率==阈值。这些变量是double类型，因此很难使用常规的等运算符进行精确比较。此外，比率变量的值是除法运算的结果。

这样的代码在像Calculator这样的应用程序中看起来特别奇怪。为了以下情况，我列出了此类警告的完整列表：

可疑的功能序列

V1020退出该函数时不调用’TraceLogger :: GetInstance（）。LogNewWindowCreationEnd’函数。检查行：396,375。计算器App.xaml.cpp 396

void App::OnAppLaunch(IActivatedEventArgs^ args, String^ argument)
{
 ....
 if (!m_preLaunched)
 {
 auto newCoreAppView = CoreApplication::CreateNewView();
 newCoreAppView->Dispatcher->RunAsync(....([....]()
 {
 TraceLogger::GetInstance().LogNewWindowCreationBegin(....); // <= Begin
 ....
 TraceLogger::GetInstance().LogNewWindowCreationEnd(....); // <= End
 }));
 }
 else
 {
 TraceLogger::GetInstance().LogNewWindowCreationBegin(....); // <= Begin
 ActivationViewSwitcher^ activationViewSwitcher;
 auto activateEventArgs = dynamic_cast<IViewSwitcherProvider^>(args);
 if (activateEventArgs != nullptr)
 {
 activationViewSwitcher = activateEventArgs->ViewSwitcher;
 }
 if (activationViewSwitcher != nullptr)
 {
 activationViewSwitcher->ShowAsStandaloneAsync(....);
 TraceLogger::GetInstance().LogNewWindowCreationEnd(....); // <= End
 TraceLogger::GetInstance().LogPrelaunchedAppActivatedByUser();
 }
 else
 {
 TraceLogger::GetInstance().LogError(L"Null_ActivationViewSwitcher");
 }
 }
 m_preLaunched = false;
 ....
}

Diagnostic V1020使用启发式方法检查代码块并查找缺少函数调用的分支。

上面的代码片段包含一个块，调用函数LogNewWindowCreationBegin和LogNewWindowCreationEnd。接下来是另一个块，只有在满足某些条件时才会调用LogNewWindowCreationEnd函数，这看起来非常可疑。

不可靠的测试

V621考虑检查’for’运算符。循环可能会被错误地执行或者根本不会被执行。CalculatorUnitTests
UnitConverterViewModelUnitTests.cpp 500

public enum class NumbersAndOperatorsEnum
{
 ....
 Add = (int) CM::Command::CommandADD, // 93
 ....
 None = (int) CM::Command::CommandNULL, // 0
 ....
};
TEST_METHOD(TestButtonCommandFiresModelCommands)
{
 ....
 for (NumbersAndOperatorsEnum button = NumbersAndOperatorsEnum::Add;
 button <= NumbersAndOperatorsEnum::None; button++)
 {
 if (button == NumbersAndOperatorsEnum::Decimal ||
 button == NumbersAndOperatorsEnum::Negate ||
 button == NumbersAndOperatorsEnum::Backspace)
 {
 continue;
 }
 vm.ButtonPressed->Execute(button);
 VERIFY_ARE_EQUAL(++callCount, mock->m_sendCommandCallCount);
 VERIFY_IS_TRUE(UCM::Command::None == mock->m_lastCommand);
 }
 ....
}

分析仪检测到一个根本不运行的for循环，这意味着测试也不会执行。循环计数器按钮（93）的初始值从开始就大于最终值（0）。

V760找到两个相同的文本块。第二个块从第688行开始.ComputerUnitTests
UnitConverterViewModelUnitTests.cpp 683

TEST_METHOD(TestSwitchAndReselectCurrentlyActiveValueDoesNothing)
{
 shared_ptr<UnitConverterMock> mock = make_shared<UnitConverterMock>();
 VM::UnitConverterViewModel vm(mock);
 const WCHAR * vFrom = L"1", *vTo = L"234";
 vm.UpdateDisplay(vFrom, vTo);
 vm.Value2Active = true;
 // Establish base condition
 VERIFY_ARE_EQUAL((UINT)1, mock->m_switchActiveCallCount);
 VERIFY_ARE_EQUAL((UINT)1, mock->m_sendCommandCallCount);
 VERIFY_ARE_EQUAL((UINT)1, mock->m_setCurUnitTypesCallCount);
 vm.Value2Active = true;
 VERIFY_ARE_EQUAL((UINT)1, mock->m_switchActiveCallCount);
 VERIFY_ARE_EQUAL((UINT)1, mock->m_sendCommandCallCount);
 VERIFY_ARE_EQUAL((UINT)1, mock->m_setCurUnitTypesCallCount);
}

另一个可疑的考验。分析仪检测到两个相同的代码片段一个接一个地执行。看起来这个代码是使用复制粘贴技术编写的，程序员忘了修改副本。

V601’false ‘值隐式转换为整数类型。检查第二个参数。CalculatorUnitTests CalcInputTest.cpp 352

Rational CalcInput::ToRational(uint32_t radix, int32_t precision) { .... }
TEST_METHOD(ToRational)
{
 ....
 auto rat = m_calcInput.ToRational(10, false);
 ....
}

该ToRational函数被调用与所述布尔值假，而相应的参数的类型的int32_t并且被称为精度。

我决定跟踪代码中的值，然后看到它被传递给StringToRat函数：

PRAT StringToRat(...., int32_t precision) { .... }

然后到StringToNumber：

PNUMBER StringToNumber(...., int32_t precision)
{
 ....
 stripzeroesnum(pnumret, precision);
 ....
}

这是目标函数的主体：

bool stripzeroesnum(_Inout_ PNUMBER pnum, long starting)
{
 MANTTYPE *pmant;
 long cdigits;
 bool fstrip = false;
 pmant=pnum->mant;
 cdigits=pnum->cdigit;
 
 if ( cdigits > starting ) // <=
 {
 pmant += cdigits - starting;
 cdigits = starting;
 }
 ....
}

该精度变量现在被命名开始，并参与表达cdigits>开始，因为这是非常可疑的假是为原始值传递。

冗余

V560条件表达式的一部分始终为true：NumbersAndOperatorsEnum :: None！= op。CalcViewModel
UnitConverterViewModel.cpp 991

void UnitConverterViewModel::OnPaste(String^ stringToPaste, ViewMode mode)
{
 ....
 NumbersAndOperatorsEnum op = MapCharacterToButtonId(*it, canSendNegate);
 if (NumbersAndOperatorsEnum::None != op) // <=
 {
 ....
 if (NumbersAndOperatorsEnum::None != op && // <=
 NumbersAndOperatorsEnum::Negate != op)
 {
 ....
 }
 ....
 }
 ....
}

该运算变量已经与值进行比较NumbersAndOperatorsEnum ::没有，所以重复的检查可以被删除。

V728可以简化过度检查。’（A && B）|| （！A &&！B）’表达式等同于’bool（A）== bool（B）’表达式。计算器Calculator.xaml.cpp 239

void Calculator::AnimateCalculator(bool resultAnimate)
{
 if (App::IsAnimationEnabled())
 {
 m_doAnimate = true;
 m_resultAnimate = resultAnimate;
 if (((m_isLastAnimatedInScientific && IsScientific) ||
 (!m_isLastAnimatedInScientific && !IsScientific)) &&
 ((m_isLastAnimatedInProgrammer && IsProgrammer) ||
 (!m_isLastAnimatedInProgrammer && !IsProgrammer)))
 {
 this->OnStoryboardCompleted(nullptr, nullptr);
 }
 }
}

这个巨大的条件表达式最初是218个字符长，但为了演示，我将它分成几行。它可以被重写为更短，更重要的更清晰的版本：

if ( m_isLastAnimatedInScientific == IsScientific
 && m_isLastAnimatedInProgrammer == IsProgrammer)
{
 this->OnStoryboardCompleted(nullptr, nullptr);
}

V524奇怪的是，’ConvertBack’功能的主体完全等同于’转换’功能的主体。计算器
BooleanNegationConverter.cpp 24

Object^ BooleanNegationConverter::Convert(....)
{
 (void) targetType; // Unused parameter
 (void) parameter; // Unused parameter
 (void) language; // Unused parameter
 auto boxedBool = dynamic_cast<Box<bool>^>(value);
 auto boolValue = (boxedBool != nullptr && boxedBool->Value);
 return !boolValue;
}
Object^ BooleanNegationConverter::ConvertBack(....)
{
 (void) targetType; // Unused parameter
 (void) parameter; // Unused parameter
 (void) language; // Unused parameter
 auto boxedBool = dynamic_cast<Box<bool>^>(value);
 auto boolValue = (boxedBool != nullptr && boxedBool->Value);
 return !boolValue;
}

分析仪检测到两个相同的功能。正如他们的名字，Convert和ConvertBack所暗示的那样，它们意味着要做不同的事情，但开发人员应该更清楚。

结论

我想每个开源的微软项目都让我们有机会展示静态分析的重要性 – 即使是像计算器一样小的项目。大公司，如微软，谷歌，亚马逊等，雇用了许多有才华的开发人员，但他们仍然是人类犯错误。静态分析工具是帮助任何开发团队提高产品质量的最佳方法之一。

图片来自 络，如有侵权，联系删除！谢谢