
Counting Bugs in Windows Calculator - julienevermind
https://habr.com/ru/company/pvs-studio/blog/443400/
======
mastax
> The analyzer has detected two identically implemented functions. As their
> names, Convert and ConvertBack, suggest, they were meant to do different
> things, but the developers should know better.

This is not a bug. Look how the `BooleanNegationConverter` is used:

    
    
        <RadioButton x:Name="fullKeypad"
         ...
         IsChecked="{x:Bind Model.IsBitFlipChecked, Converter={StaticResource BooleanNegationConverter}, Mode=TwoWay}"/>
    
        <RadioButton x:Name="bitFlip"
         ...
         IsChecked="{x:Bind Model.IsBitFlipChecked, Mode=TwoWay}"/>
    

Two radio buttons bound to the same boolean value, one with the negation
converter. If `fullKeypad` is checked, `IsBitFlipChecked` should be set to
false. If `IsBitFlipChecked` is set to true, `fullKeypad.IsChecked` should be
set to false. The converter needs to do the same operation in both directions.

~~~
taco_emoji
Agree it's not a bug as I assume it's an implementation of the IValueConverter
interface: [https://docs.microsoft.com/en-
us/dotnet/api/system.windows.d...](https://docs.microsoft.com/en-
us/dotnet/api/system.windows.data.ivalueconverter?view=netframework-4.7.2)

However since the method bodies are identical, IMO one should have the full
implementation and then the other should just call the first. Still not a
_bug_ , just a code smell.

------
geofft
Is this PVS-Studio blogspam? They're always so proud of being able to detect
the same things gcc/clang -Wall -Wextra detects.

~~~
AndreyKarpov
PVS-Studio vs Compilers:

Clang: [https://www.viva64.com/en/b/0108/](https://www.viva64.com/en/b/0108/)
, [https://www.viva64.com/en/b/0155/](https://www.viva64.com/en/b/0155/) ,
[https://www.viva64.com/en/b/0446/](https://www.viva64.com/en/b/0446/)

GCC: [https://www.viva64.com/en/b/0425/](https://www.viva64.com/en/b/0425/)

~~~
acdha
Do you have Clang tests which are more recent than 8 years ago? Or,
especially, any which were validated and accepted upstream?

~~~
AndreyKarpov
The last check was in 2016:
[https://www.viva64.com/en/b/0446/](https://www.viva64.com/en/b/0446/) . Yes,
it was few years ago because we were not interested in writing the same
articles again and again. There are a lot of other interesting projects in the
world:
[https://www.viva64.com/en/inspections/](https://www.viva64.com/en/inspections/)
. I also want to note that you assume that Clang was worse than PVS-Studio
long time ago but now it can catch up. To that, I will say that all these
years we’ve been working on analyzer development :). By the way, here is where
you can read about the way it works: "Technologies used in the PVS-Studio code
analyzer for finding bugs and potential vulnerabilities" \-
[https://www.viva64.com/en/b/0592/](https://www.viva64.com/en/b/0592/)

~~~
acdha
More simply, I'm just curious about what, as a developer in 2019, would be the
selling point for using it if you were on a project which was relatively clean
using Clang's warnings and static analysis toolchain, flawfinder, etc.

------
sonofgod
In 'Suspicious comparison of real numbers':

isActive = ((ratio > threshold) || (ActiveIfEqual && (ratio == threshold)));

is called out as bad code by the blogger; this feels very much like a false
positive, given that we're checking an inequality already; just effectively
changing it from > to >=

A good example of not just blindly following the linter...

~~~
scscsc
Still, the comparison ratio == threshold has very little chance to turn out
true.

When you want to check whether A >= B (where A and B are numbers represented
as floats), you would probably want to use the check A >= B - epsilon.

~~~
melkiaur
I disagree strongly. It looks obvious to me that they can be equal and that
this is a special case. The comparison is done only after checking for A<B,
and only if a boolean called "ActiveIfEqual" is true. I mean, I don't know how
much more obvious they could have made it.

------
gitgud
It's interesting to see the transition from the old _Proprietary Microsoft_ ,
to the new _Open-Source Microsoft_. Opening up code like this, which is
unpolished and has bugs, is a sign of good will and transparency which I
appreciate.

Although considering this application _Calculator_ could be one of the most-
used programs on Windows, and they left that many errors in. It's scary to
think how many errors could be in other parts of the system...

~~~
anticodon
BTW, I think that python/ipython cli is a much better calculator. I stopped
using other calculators long time ago. Although I understand that it's
probably too complicated for ordinary user.

~~~
Beltiras
There is no task executed in Excel that I can't do better and/or faster in
ipython.

~~~
thanatropism
\- Pivot Tables

\- Reach goal.

\- Simple curve fitting

\- Drawing charts in general (even with matplotlib categorical bat charts are
a world of pain and then they tell you to learn Altair or something.

\- Tables with conditional formatting

\- ...

------
nerdbaggy
I think all the code errors like this is why some people are scared to open
source their code. While it’s a great learning experience some people just get
overwhelmed with the possibility of wrongness so they don’t.

~~~
uranusjr
I’d go as far as saying that this kind of posts worsen the situation. In
general, if you find bugs in open source code, it’s better to quietly file
bugs to the author instead of writing a full post listing errors people made.
It’s like your high school teacher calling you out in front of class for
mistakes you made in a test. Not cool.

~~~
daveFNbuck
This post was an advertisement for static code analysis, not a shaming. It
even mentions that these bugs will probably soon be fixed, which is good for
Microsoft.

~~~
melkiaur
This post shows some false-positives and present them as bugs. It is mildly
disingenuous for the casual observer.

------
zamalek
_> Suspicious function sequence_

As far as I remember, and it has been a while (and is virtually undocumented),
with ETW the convention is to _not_ call the EndX method when an error occurs
as EndX is basically synonymous with `return`. This might have been carried
over.

Either way, I'm sure there are false positives, as always with static
analysis.

------
palotasb
This seems like a stolen article. The URL should be changed to the original
blog post:
[https://www.viva64.com/en/b/0615/](https://www.viva64.com/en/b/0615/)

~~~
golergka
They usually repost their articles on Habr themselves in their corporate blog,
linking to the source. It's a very popular Russian IT resource, so they do it
to promote their articles, nothing is stolen, it's just reposted.

------
Latteland
Thanks for the pointer, that was actually interesting. No one looks at most
old code bases unless they are changing something in particular, that's how
old bugs hang around forever. Commercial software companies of the world, open
source your old crappy utilities and the world may improve them. Of course you
don't do that because you are embarassed at their poor quality. Based on my
own personal experience :-). You should have seen the source code of sybase
sql server.

~~~
Narishma
This isn't an old code base though. It's the new UWP calculator that was
introduced in Windows 10 (or was it 8?).

~~~
acdha
This is the current codebase but it goes back at least 20 years in places. If
you look at the core engine you'll see plenty of comments dating back to 1995:

[https://github.com/Microsoft/calculator/search?q=1995&unscop...](https://github.com/Microsoft/calculator/search?q=1995&unscoped_q=1995)

------
showdead
Calculator is interesting in that there is a physical button on (many)
keyboards to open it. Ironically enough, I encountered a bug with the Windows
10 calculator that caused it to crash upon opening: the window would draw,
then a second later vanish.

Turned out that it was due to something profile related: re-registering the
Windows App Store fixed the issue. Yes, that's right, a dependency on the app
store can prevent you from running Calculator.

------
julienevermind
There's another article on calculators' code check. This time is Qalculate - a
multi-purpose cross-platform desktop calculator.
[https://habr.com/en/company/pvs-
studio/blog/443656/](https://habr.com/en/company/pvs-studio/blog/443656/)

------
tonyedgecombe
It’s interesting that most of these would have been avoided by using a safer
language than C++.

~~~
renholder
Calculator has been around since ninteen-eighty-something-or-other[0] and this
seems to be a revamp of the base code, with a XAML (read: WPF) wrapper[1].

Aside from the obvious answer of them using C++ based upon "using something
you know and that works", why _shouldn 't_ they have used C++? (Genuinely
wanting to know/understand.)

It would seem like a monolithic task to rearchitecture the entire app versus
just dropping the code-behind into a new UI wrapper, yeah? I'm not excusing
it, to be sure, just explaining the potential ROI perspective they might've
had when they stuck with C++.

[0] -
[https://en.wikipedia.org/wiki/Windows_Calculator#History](https://en.wikipedia.org/wiki/Windows_Calculator#History)

[1] -
[https://en.wikipedia.org/wiki/Windows_Calculator#Windows_10](https://en.wikipedia.org/wiki/Windows_Calculator#Windows_10)

~~~
ygra
The code base has changed quite a bit since 198x:

\-
[https://blogs.msdn.microsoft.com/oldnewthing/20040525-00/?p=...](https://blogs.msdn.microsoft.com/oldnewthing/20040525-00/?p=39193)

\-
[https://blogs.msdn.microsoft.com/oldnewthing/20160628-00/?p=...](https://blogs.msdn.microsoft.com/oldnewthing/20160628-00/?p=93765)

\-
[https://blogs.msdn.microsoft.com/oldnewthing/20180704-00/?p=...](https://blogs.msdn.microsoft.com/oldnewthing/20180704-00/?p=99165)

This might also explain the choice of C++. Rather than rewriting their
calculation engine in another language (it works, so a rewrite is rarely a
good idea), UWP (not WPF; XAML is merely an XML dialect for object graphs and
works pretty much anywhere if you want it to) allows you to write the UI in
C++ (C++/CX to be precise) as well, and the UI code is for the most part not
that complicated that C++ is a major impediment. That, and of course, most
developers for Windows applications at Microsoft are familiar and well-versed
in C++. Rewriting it in JavaScript or C# may take longer.

~~~
gardaani
It will be interesting to see how soon they will convert it to C++/WinRT
because C++/CX will be retired at some point.

 _The first step toward full C++ /WinRT support within Visual Studio and the
eventual retirement of C++/CX happened today with the availability of build
17025 of the Windows SDK Insider Preview._

[https://moderncpp.com/2017/11/01/cppwinrt-in-the-windows-
sdk...](https://moderncpp.com/2017/11/01/cppwinrt-in-the-windows-sdk/)

