Logical Expressions in C/C++. Mistakes Made by Professionals
-
In the beginning of this year, I used the PVS-Studio analyzer to scan some projects by large IT companies, which, following the modern trend, make their projects' sources publicly available under free licenses. I started to notice that almost every project has errors in conditional expressions that deal with incorrect use of conditional operators. Expressions themselves are quite simple and consist of just three operators:
• != || !=
• == || !=
• == && ==
• == && !=In total, you can write 6 conditional expressions using these operators, but 4 of them are incorrect: two are always true or false; in two others, the result of the whole expression does not depend on the result of one of its subexpressions.
For example, the following is a synthetic example where the conditional expression will always evaluate to true:if ( err != code1 || err != code2)
{ …. }Mistakes described in my article are the ones made by the developers of such well-known projects as FreeBSD, Microsoft ChakraCore, Mozilla Thunderbird, LibreOffice, and many others.
-
@Alticor said in Logical Expressions in C/C++. Mistakes Made by Professionals:
For example, the following is a synthetic example where the conditional expression will always evaluate to true:
But can the author of the code prove that at the time that it was written? This is an issue where some values (e.g., system error codes) are the same on some operating systems yet different on others; you might be analysing things on a platform where the value is silly, but the test is actually testing something important elsewhere.
Though usually it's the mark of a doofus.
-
This post is deleted!
-
We can prove that the logical expressions are incorrect by building truth tables. Such errors aren’t related to the operational system; moreover similar errors can be made in other languages, not only C/C++.
-
@Alticor What if
code1
andcode2
are the same on System A and only different on System B?In total, you can write 6 conditional expressions using these operators, but 4 of them are incorrect
This doesn't make any sense. The example you provided does shine light on what you're talking about but I'm not sure why you're trying to extract a general truth from it?
if(var1 != var2 || var3 != var4)
is perfectly fine in most situations.
-
@Alticor said in Logical Expressions in C/C++. Mistakes Made by Professionals:
We can prove that the logical expressions are incorrect by building truth tables
Assuming code1 != code2. @dkf was saying that these patterns could come up when it's possible for them to be the same
-
@Alticor Are these "mistakes" actual bugs or you just being pedantic about half-a-line's worth of unreachable code?
Because I'm finding it really hard to give a shit based on what you've written here.
-
@blakeyrat +1. If there isn't an actual bug or vulnerability exposed, MEH. What the real news here is that these projects aren't doing static analysis (or aren't zeroing out the reports).
-
@Weng said in Logical Expressions in C/C++. Mistakes Made by Professionals:
What the real news here is that these projects aren't doing static analysis (or aren't zeroing out the reports).
... or that they did the static analysis and they only bothered to fix the stuff that actually mattered.
So no, it can't even be used to prove that point.
-
@blakeyrat There's an argument for cleaning up some of that stuff: It makes the code more legible, reducing the likelihood of inserting more bugs in future.
-
Please, read the full article to see more details. I've provided there examples of four error patterns that I detected in such projects as FreeBSD, Microsoft ChakraCore, Mozilla Thunderbird, LibreOffice and some others. These are examples of real bugs in real projects.
-
There is no error in your example. The variable must be the same, but constants - different.
if(var != var1 || var != var2)
Take a look at all 4 examples in the article.
-
@blakeyrat this is the first example he gave in the article. Obviously a bug. Check if the symbol is null or empty, then go ahead and insert it in the cache regardless.
if( (pSymDef->GetType() != SbxEMPTY) || // <= (pSymDef->GetType() != SbxNULL) ) // <= aCache.InsertGlobalVar( pSymDef->GetName(), pParser->aGblStrings.Find(pSymDef->GetTypeId()) );
-
@Alticor said in Logical Expressions in C/C++. Mistakes Made by Professionals:
== || !=
I've seen
if (var == code1 || var != code1) {...}
in production code before.
-
This post is deleted!
-
@CarrieVS said in Logical Expressions in C/C++. Mistakes Made by Professionals:
if (var == code1 || var != code1) {...}
I've seen stuff like that before. Generally assumed it was debugging code that never got removed
-
@Buddy said in Logical Expressions in C/C++. Mistakes Made by Professionals:
@blakeyrat this is the first example he gave in the article.
I'm not reading the damn article.
Did he come here to participate in a forum, or get hits to make the big buxxx from his blog's ad revenue?
-
@Alticor said in Logical Expressions in C/C++. Mistakes Made by Professionals:
such projects as FreeBSD, Microsoft ChakraCore, Mozilla Thunderbird, LibreOffice and some others. These are examples of real bugs in real projects.
Oh you mean open source software is buggy as shit? What an insight.
-
@blakeyrat said in Logical Expressions in C/C++. Mistakes Made by Professionals:
get hits to make the big buxxx from his blog's ad revenue?
Turn on adblock before you visit, then
-
@blakeyrat As opposed to closed-source software, whose buginess is hidden from public view?
-
@Jaloopa said in Logical Expressions in C/C++. Mistakes Made by Professionals:
Generally assumed it was debugging code that never got removed
I don't believe it was in this instance. It seems that we had had a rule in the past to do something only if a certain condition. Then we got a requirement to scrap that rule and do the stuff regardless, and that was the solution.
-
@CarrieVS Ah, the old "must not remove code" antipattern.
-
@Alticor said in Logical Expressions in C/C++. Mistakes Made by Professionals:
if ( err != code1 || err != code2) { …. }
It seems like most of the people responding are completely missing the bug in that expression. Although it appears to me that it is more like a typo, where they likely meant && not ||? Or maybe some programmers just completely fail at truth tables and really thought || was what they wanted.
-
@blakeyrat He posts this articles on other programing forums too, It's a way to promote their static analyzer. Sometimes the articles area good, sometimes amusing, didn't read this one yet to say if it's worth the read.
@Alticor You'll have far less luck here than on gamedev.net, people here are far more pedantic and far less interested in work stuff, but by all means try it.
@dkf there has been examples of false positives on his articles before, but the only ones I'm aware off are worth of an article on CodeSOD.
-
@Sentenryu said in Logical Expressions in C/C++. Mistakes Made by Professionals:
He posts this articles on other programing forums too, It's a way to promote their static analyzer.
Ah, so he's shilling
-
@CarrieVS said in Logical Expressions in C/C++. Mistakes Made by Professionals:
Then we got a requirement to scrap that rule and do the stuff regardless, and that was the solution.
They're only errors if you don't understand what the outcome is.
Assuming b != c a != b || a != c a = b | a != b +-------+-------+ a = c | X | T | +-------+-------+ a != c | T | T | +-------+-------+ This can be simplified to: true; a == b && a == c a = b | a != b +-------+-------+ a = c | X | F | +-------+-------+ a != c | F | F | +-------+-------+ This can be simplified to: false; a == b || a != c a = b | a != b +-------+-------+ a = c | X | F | +-------+-------+ a != c | T | T * | +-------+-------+ This can be simplified to: a!=c. a == b && a != c a = b | a != b +-------+-------+ a = c | X | F | +-------+-------+ a != c | T | F * | +-------+-------+ This can be simplified to: a==b.
-
This whole topic can be summarized as: "read my article". At least you could have made a YT video about it, like us cool kids do nowadays.
-
@NeighborhoodButcher Don't give them ideas. They'll do it.
-
@NeighborhoodButcher Yeah a big-ass video embed would have been slightly more difficult to ignore.
Don't get me wrong, I'd still ignore it. But just sayin'.
-
@blakeyrat said in Logical Expressions in C/C++. Mistakes Made by Professionals:
Yeah a big-ass video embed would have been slightly more difficult to ignore.
Only by nanometers. WTDWTF are world experts at ignoring stupid shit.
-
@Sentenryu I don't plan to actually read the article to find out if it is a false positive or not. I'm just pointing out that, in C at least, having two arms of a
&&
or||
be the same might be a consequence of macro expansion and so not actually a bug.
-
-
Resharper seems to detect the same type of errors. What does your tool do better?
The article was reasonably informative, but hardly surprising.
-
Oh look, a wtfvertisment!
-
@dkf Their static analyzer runs before macro expansions. I actually read the article now and it doesn't look like it has any false positives.
It does have an example on Chakra that really demonstrates his point:
if ((str[i] != '/' && str[i] != '-') || (sub[i] != '-' && sub[i] == '/')) { / <= // if the mismatch is not between '/' and '-' break; }
I guess it's a bug and the second conditional should use
!=
?Anyway, I think his advertisement is a little bit less obnoxious than most because they actually have some content to show. On gamedev.net they analyze open source games and game engines, as well as some graphics libraries, they even have an article on doom.
I find this one a better read: http://www.viva64.com/en/b/0385/
Also, they did check their own code: http://www.viva64.com/en/b/0382/
-
@dkf said in Logical Expressions in C/C++. Mistakes Made by Professionals:
@blakeyrat said in Logical Expressions in C/C++. Mistakes Made by Professionals:
Yeah a big-ass video embed would have been slightly more difficult to ignore.
Only by nanometers. WTDWTF are world experts at ignoring stupid shit.
No we aren't. we are world experts at calling out stupid shit, pointing and laughing at it, then arguing why we all think it is stupid then pointing and laughing at each other until the next stupid shit arrives.
-
if( oneMan.stupidShit == anotherMan.awesomeSauce || anotherMan.stupidShit ==
...eh...making this metaphor work here is too much stupid shit even for me.
-
Incorrect use of the ?: operator
This is pretty much why I've adopted explicit order of operations.
() around all the things.
-
@boomzilla said in Logical Expressions in C/C++. Mistakes Made by Professionals:
work here is too much
There, that's more concise and evaluates the same.
Filed under: Static post analysis
-
@blakeyrat said in Logical Expressions in C/C++. Mistakes Made by Professionals:
Did he come here to participate in a forum, or get hits to make the big buxxx from his blog's ad revenue?
@NeighborhoodButcher said in Logical Expressions in C/C++. Mistakes Made by Professionals:
This whole topic can be summarized as: "read my article".
No, it's worse that that.
His blog is a giant whitepaper for his software, masquerading as a blog of programming topics.
@Sentenryu said in Logical Expressions in C/C++. Mistakes Made by Professionals:
Anyway, I think his advertisement is a little bit less obnoxious than most because they actually have some content to show.
No, it's more obnoxious, because the content is rather trivial, but up-sold as introspective.
Here's the least apologetic, most ego-stroking one An always up-to-date list of articles describing errors that we find in open source projects with PVS-Studio analyzer
Then they go on to rip up other people's software unwarranted, just to sell their product.
http://www.viva64.com/media/images/content/b/0389_OpenToonz/image9.png
It'd be different if the blog was independent, and on relevant articles he linked to his software. But it lives on the product-site, and every article is eventually tied to code-analyzers, no matter how diverse the titles.
It's the reason I stopped reading coding-horror.
-
If this topic was honest and said ,
"I make this great software, and I'm just asking people to take a look and see if they would like it."
And it was from a user that contributed and knew the community, I might actually have taken it seriously.
But now, I'm of the opinion that sleazy marketing means a sleazy product.
-
@xaade I'm not sure why they expect any return from advertising here.
His articles are, indeed, always focusing on their code analyzer, but I haven't seen any so agressive as the one you linked. Then again, it's been a long time since I looked over at gamedev.net
There his articles are better placed in that they help raise awareness to the new programers that are trying to create games with little or no knowledge of C++, but I'm not sure they will add anything here.
Sadly, the WTF density on those articles is far too low for us.
Might be worth to explicitly prohibit advertisements here and just delete those posts, but then we will need a "sticky" thread on the main page with the forum rules, somewhere easy for newcomers to see.
-
I don't like rules.
I prefer guidelines.
He's free to post here. Being taken seriously is another issue.
-
@xaade said in Logical Expressions in C/C++. Mistakes Made by Professionals:
He's free to post here. Being taken seriously is another issue.
If he's putting the link up for the SEO, he may not need us to be serious. Unless google's gotten smart enough to realize our sentiments on that link are negative.
-
@PleegWat Or smart enough to realize we are aways negative on anything posted here
-
@Sentenryu said in Logical Expressions in C/C++. Mistakes Made by Professionals:
Might be worth to explicitly prohibit advertisements here and just delete those posts, but then we will need a "sticky" thread on the main page with the forum rules, somewhere easy for newcomers to see.
Fuck that.
-
@Sentenryu said in Logical Expressions in C/C++. Mistakes Made by Professionals:
new programers that are trying to create games with little or no knowledge of C++
Those sorts of people should not be using C++. Unity would be a much better fit for these people to get their feet wet with respect to learning how to code without having to worry about object ownership/lifetimes and all the security concerns.
And I say this as someone whose pet project is in C++ and who thinks Unity is a gigantic steaming turd. I see tons of ads for "Learn to code games in C++ using Unreal in X days!" which make me smile, 'cause that shit takes years to master.
-
@blakeyrat said in Logical Expressions in C/C++. Mistakes Made by Professionals:
@Sentenryu said in Logical Expressions in C/C++. Mistakes Made by Professionals:
Might be worth to explicitly prohibit advertisements here and just delete those posts, but then we will need a "sticky" thread on the main page with the forum rules, somewhere easy for newcomers to see.
Fuck that.
Pretty much. If we went that route, we would probably need ten or twenty stickies to explain all the in - jokes and community etiquette, which would all get derailed. Besides, part of the fun in this community is that we have almost no rules.
-
@Groaner Unreal Engine hasn't used C++ in ages.
(Well, I suppose they still sell a source license and in theory you could buy that and patch their engine which would require the use of C++... but you know what I mean.)
-
@Groaner Yeah, that's the general advice given there, but some people use pet games as a way to learn the language. I find that acceptable.
Also, with Unreal 4 you don't need to know C++ to make games. As a matter of fact, even knowing C++ doesn't help that much, seeing as epic employ tons of macros to do stuff such as showing properties on the editor for the programmer. The Unit C++ looks like a totally different beast than standard C++