Can you do that in a single file – assuming the latter errno is the one defined in errno.h (or cerrno). I was of the impression that errno may also be a macro, so could you say it's a good recommendation to have it as an argument's name – or in general even to reuse the name anywhere?
I agree that the option (of passing the helper function no arguments at all) is not perfect either, but... (I also don't like that you need to call your stored errno's errnums or the like, but that's what we have.)
If errno can be a macro, as at least one man page claims, the argument in the function could expand and even not compile or produce something unwanted. I'd prefer:
errno is a macro on most (maybe every) platform supporting pthreads.
Furthermore, C11 literally defines errno as a macro in 7.5p1:
The macros are
EDOM
EILSEQ
ERANGE
which expand to integer constant expressions with type int, distinct positive values, and
which are suitable for use in #if preprocessing directives; and
errno
which expands to a modifiable lvalue that has type int and thread local storage
duration, the value of which is set to a positive error number by several library functions.
If a macro definition is suppressed in order to access an actual object, or a program
defines an identifier with the name errno, the behavior is undefined.
Notably, C11 introduced an optional threading API, effectively derived from the common functionality between Window's threading API and POSIX pthreads. While I can imagine ways to have a unique errno symbol that doesn't rely on macros, the obvious solution does and there's little reason not to use it.
While C11 is most definitely not the same thing as C++11, I think, similar to atomics, the committees cooperated on the threading API. I don't have a recent C++ standard handy at the moment, but I wouldn't be surprised if C++11 at least allowed defining errno as a macro.
You'd think. But there was a long argument here[1] not even a week ago with multiple people passionately asserting that they were entitled to be able to rely on completely predictable and non-fatal semantics when derefrencing a null pointer.
The problem with C and C++ isn't so much that undefined behavior exists, but the fact that the rules that determine when a program has defined behavior aren't formalized rigorously enough. The C and C++ specifications are written in natural language, which always leaves room for ambiguity, no matter how pedantically one tries to write.
If you want to convince a single other person that a C program you have written has no undefined behavior, then you and that person have to hopefully interpret English the same way. It would be completely hopeless to try to convince a hundred people that a nontrivial C program has no undefined behavior. And successful programs typically have way more than a hundred users.
The phasing was chosen to be provocative and attention grabbing (because, yeah, duh). The content of the point, however, is about avoiding undefined behavior, period, and not just lazily assuming "it works" means "it will always work".
This used to compile and work just fine years ago (perhaps relying on undefined behavior):
static void takes_null_reference_by_default(int& p = *((int*) nullptr)) {
if (&p) ++p;
}
int main(int argc, char* argv[]) {
int v1 = 15;
int v2 = 25;
takes_null_reference_by_default(v1);
takes_null_reference_by_default();
takes_null_reference_by_default(*((int*) nullptr));
takes_null_reference_by_default(v2);
std::cout << v1 << " " << v2 << std::endl;
}
Now g++ complains with: junk.cpp:5:10: warning: reference cannot be bound to dereferenced null pointer in well-defined C++ code; pointer may be assumed to always convert to true
[-Wundefined-bool-conversion]
./a.out still runs though. And the de-references are just pseudo-de-references, it never "really" happens.
EDIT: also mentioning the fits "delete this;" gave some people, but worked very well in the ACE Networking Library and various other places.
Yes, it is an ad for PVS-Studio but I don't think that should tarnish the content of code discussion in the article.
Although I'm extreme anti-advertisement (with dedicated firewall to block web ads), I find this article having 3 qualities that's valuable:
1) instead of showing contrived bad code nobody writes, it shows real bugs from real projects such as MySQL, PostgreSQL, ReactOS, Apache server, Audacity, LibreOffice, Notepad++, CoreCLR, etc
2) full explanations of the bug and further advice for correcting the code
3) learning is free: you don't have to buy PVS-Studio to apply the solutions learned from the article or gain the ability to spot similar bugs later
That all adds up to a quality article which is better than the old PC-Lint ads[1] from Gimpel Software.
For those too young to be aware of them, in the 1990s, GS ran ads in a magazine called C/C++ Users Journal. For the others that enjoyed the "puzzles" in those ads, Andrey Karpov's article is a fine continuation of that.
Funny how the "proper" first example still has a lot of things to be addressed, like the fact that it has a magic number (7) coming from nowhere, which is duplicated a few times as well.
It would be way less error-prone to calculate the number of elements using sizeof, for instance. This would also make the function independent of the type (a template could be used), thus reusable in another contexts.
Item 5 is interesting, but I think it might be worthwhile to include the entire function here (including the comments):
BOOL WINAPI DllMain( HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved )
{
(void)hinstDLL; /* avoid warning */
(void)lpvReserved; /* avoid warning */
switch (fdwReason)
{
case DLL_PROCESS_ATTACH:
{
TCHAR szBuffer[64];
// This code will attach the process to its parent process
// if the parent process had set the environment variable.
// The corresponding code (setting the environment variable)
// is desktop/win32/source/officeloader.cxx
DWORD dwResult = GetEnvironmentVariable( "ATTACHED_PARENT_PROCESSID", szBuffer, sizeof(szBuffer) );
if ( dwResult && dwResult < sizeof(szBuffer) )
{
DWORD dwThreadId = 0;
DWORD_PTR dwParentProcessId = (DWORD_PTR)atol( szBuffer );
if ( dwParentProcessId && GetParentProcessId() == dwParentProcessId )
{
// No error check, it works or it does not
// Thread should only be started for headless mode, see desktop/win32/source/officeloader.cxx
CreateThread( NULL, 0, ParentMonitorThreadProc, (LPVOID)dwParentProcessId, 0, &dwThreadId );
// Note: calling CreateThread in DllMain is discouraged
// but this is only done in the headless mode and in
// that case no other threads should be running at startup
// when sal3.dll is loaded; also there is no
// synchronization with the spawned thread, so there
// does not appear to be a real risk of deadlock here
}
}
return TRUE;
}
case DLL_THREAD_ATTACH:
break;
case DLL_THREAD_DETACH:
osl_callThreadKeyCallbackOnThreadDetach( );
break;
}
return TRUE;
}
The title is very misleading. I was expecting a discussion about the future of programming, why there will probably never be a "best" language or why we're not all using functional languages yet. Was disappointed after reading the first sentence:
> Yes, you've guessed correctly - the answer is "42". In this article you will find 42 recommendations about coding in C++ that can help a programmer avoid a lot of errors, save time and effort.
The article states that these tips are 'usually universal', but I hardly found that to be true, only having counted about 10 points not being about C++.
The title should probably be something like the obvious "42 tips for C++ programmers" as the current one really doesn't justify the content.
I agree the title originally as submitted[1] is not helpful. When the author submitted[2] it to Intel DZ, he re-used the same vague title as the original PVS-Studio article from April 2016: http://www.viva64.com/en/b/0391/
HN admins already edit titles with [YEAR] to signify an old article. In the same vein, I do wish the admins (or the original submitters to save the admins the effort) went even further and exercised editorial judgement to add [NON-CUTE SUBTITLE] to negate the effects of cute-but-meaningless titles.
In this case, there would be a "[C/C++ refactoring tips]" like you suggested appended to the end of the title.
[1] original submitted HN title: "The Ultimate Question of Programming, Refactoring, and Everything"
You're very much correct, but I'm almost glad I got hoodwinked. I've debated whether I'm handicapping myself by sticking with Python and Rust, but reading through this list I was just aghast at how many problems I didn't realize I was avoiding.
Why stop there? Your summary boils down to "just exist".
The linked article is incredibly detailed, much more so than 90%+ of articles that pass through HN. It talks about nuances, e.g. that copy-paste mistakes more than likely happen on the last line as opposed to any other line. Claiming it can be replaced with a two word phrase is wildly incorrect.
Indeed, however every programmer no matter how good will make mistakes sometimes. It can be in 1%, 0.1%, 0.01% of LoC. Loosing attention can be so easy, it can be a co-worker walking by, an important email/meeting happening while you were focused on a 'copy-pasting' step.
Having a tool that is trained to spot this kind of human errors is good. It can be useful in any language, not only C++. Avoiding them in the first place and giving you advices on how to not do it again would be a nice thing compiler could provide. i.e:
a[0] = b[0];
a[1] = b[1];
a[2] = b[0];
^ warning: Are you sure you want to assign b[0] to a[2]. If not, consider rewriting your code with a for loop
a[3] = b[3];
The author of the article actually develops the tool that can help the programmers detect such errors. The compilers don't report such kind of errors because it was traditionally to expensive to detect them. The special tool, like the one mentioned in the article, doesn't have to produce the optimized code and doesn't have to be run as frequently as on each compilation but it can allow the programmer to detect potential problems.
Seeing that list of error patterns made me realize that letting mediocre programmers use something like C / C++ really boils down to Luck-Driven Programming. I hope that in the not too distant future something along the lines of Rust or Swift can help avoid at least some of those traps.
There is no silver bullet. And no language can stop you from making mistakes (assuming “at least some” means the same as “all” is one example). But some languages help you avoid certain classes of mistakes with automated checks. Like Rust.
And even if you added those features to C++, you couldn't guarantee a codebase that only uses the secure subset. Nobody has the time to rewrite old code. And making sure that current code uses only the secure subset would require code reviews. Not everyone does that.
Rust and Swift have sum types, so their equivalent of `memcmp` can have a return type with exactly three values (`Less`, `Equal`, `Greater`).
> 3. Copy once, check twice
I'd like to see the context of this code. Perhaps this could have been written more cleanly using iterators.
> 4. Beware of the ?: operator and enclose it in parentheses
Not a problem in Rust. `if cond { true_case } else { false_case }` is an expression in the obvious way.
> 5. Use available tools to analyze your code
The right tool is called “type checker”, but of course, the language has to be intelligently designed, rather than just evolved, for the type system to be useful.
> 6. Check all the fragments where a pointer is explicitly cast to integer types
Or, even better, don't do it at all.
> 7. Do not call the alloca() function inside loops
Or even better, don't do it at all.
> 8. Remember that an exception in the destructor is dangerous
Rust solves this neatly: use different tools for recoverable and unrecoverable errors (the `Result` sum type and panics, respectively). Destructors always return `()`, so they can't fail in a recoverable way.
> 9. Use the '\0' literal for the terminal null character
Or even better, use Rust-style iterators and forget this null-termination nonsense.
I don't understand why you got downvoted for citing facts. It seems, some readers in this thread do everything they can to not see the elephant in the room.
I think they but downvote for this comment because this approach is not very sensible. There is no point to say "you should write well and without mistakes". Moreover there is no point to recommend rewriting everything in a different language - it is impossible for large projects.
> There is no point to say "you should write well and without mistakes".
That's not what I'm saying. I'm showing how better-designed languages (which, to be fair to C++, have the benefit of over 20 years of hindsight) avoid some of the pitfalls you list.
If you're an experienced C++ coder, I'd skip this article.
While it's well structured, with real-world examples, it's very basic. I'm not an excellent C++ coder by any stretch of the imagination. I make lots of mistakes. But not these. The article is fairly thorough and long, so if you're experienced you'll invest non-trivial time for little or no reward.
Given the errors found by PVS-Studio on actual relevant FOSS projects, I would say it is a good introduction for looking into such examples, for those interested in writing proper code.
The fact that these are simple errors doesn't mean that these are not errors. We should understand tat they are especially noticeable when there is a small code fragment. In the code of a real application such errors can live for years. By the way, I recommend playing: http://q.viva64.com/
Every one of these points is just "use a static analyzer" and while I appreciate that I definetly don't appreciate them pushing Microsoft's static analysis tooling rather then Free tooling.
I'd honestly welcome a post about different static analysis tooling that's libre and open source. Their pros and cons as well as how to deploy them would be good too.
PVS is a product that performs a service. They have made major efforts to port this product to Linux; they have multiple articles scanning Linux FSF products and the Linux Kernel.
People deserve to be paid for their work if it's useful.
I'm sure Andrey would be willing to give you the source code, or even just straight out GPL/BSD license it to everyone if you paid enough. So you see, it still comes back to the problem that you aren't willing to pay what is requested.
The author of this article is Andrey Karpov. He develops his own static analyzer called PVS-Studio. This article was originally published on his company's website: http://www.viva64.com/en/b/0391/. If anything, he is pushing his own (proprietary) static analyzer, not Microsoft's one.
A lot of the PVS-Studio analyses are described in enough detail that if someone wanted to, they could implement them as clang checkers. clang is definitely open source, I don't know about "libre" though.
Wow, the first example is simply a typo as a result of copy and pasting the same conditional block repeatedly. You would learn this would cause problems in your first week of programming: that what you type in your code determines whether or not it will perform correctly.
Shouldn't they focus on examples of unexpected or non-obvious mistakes (or at least start with those -- but then why add obvious examples at the end)?
Whether you copy and paste or simply type a single line incorrectly, of course if the wrong thing is entered the code will simply be incorrect.
Of-course these are obvious errors once you are given 10-15 lines of code and told that there is an error there. Imagine having this code in the middle of say, several million loc.
Anyways the argument the original author is trying to make here is, someone did this to improve performance by "unrolling" the loop, it is a good example of how you shouldn't do things that a compiler might be able to for you, to try and prioritize making code human readable, since your compiler is smart enough to figure out these micro enhancements.
You're missing the point: repeated blocks of code (whether they are copy+pasted or typed out) are simply bad, not from a computing point of view, in terms of what instructions get executed when, but from a human point of view, in terms of how we read and comprehend code. It's dealing with a different aspect of computer programming, rather than the obvious 'what works, what's efficient', but that makes it more interesting, not less.
Aren't switch-case statements widely accepted in C++ and inherently repeated blocks of code? No one would tell you to not copy-paste and then edit just the different parts in a switch-case block.
And to support both our arguments -- that example first mentions copy-pasting being the likely cause of the error and then provides a rewritten example that doesn't use the same code repeatedly (your point).
But a switch-case statement is a counter-example that you would copy-paste code and isn't abstracted into variable array indexes.
Counter-counter-argument: switch-cases have more obvious differences in a (only potentially) multi-character string.
No, they're not inherently repeated blocks of code. You can repeat bits of code if you like, but for many common cases you have options.
1. both cases have identical code: just fall through from one to the next
case A:
case B:
// code for A and B
break;
2. one case's code is a prefix of the other: have the first case first, let it do its thing, then fall through to the second case
case A:
// code for A only
case B:
// code for A and B
break;
3. both cases have a shared suffix; have them separate, then use a goto to get to the suffix part.
case A:
// code for A only
goto AB;
case B:
// code for B only
AB:
// code for A and B
break;
4. code is somehow parameterized by the switched expression - just reuse it in the code however you need to. This is the most flexible, of course, as it lets you do anything you like.
switch(x) {
case A:
case B:
printf("value is: %d\n",important_array[x]);
if(x==A) /* something */;
} else if(x==B) /* something else */;
Both of those statements can be true, though. Switch-case statements are widely accepted in industry, yet some developers disagree with their use. In my opinion, it's hard to beat a jump table for some operations, which a switch block neatly performs. And there's a rational continuum between boolean conditionals, multi-state (>2) conditionals, and polymophism, such that i think "some people"'s assertion sounds like throwing out the baby with the bathwater.
Copy-paste errors are much more common than those once in a lifetime bugs nobody care about. Or are you telling me you never ever made a copy-paste error after your first week of programming?
They are one of the most pernicious error to find because the person reading is doing the exact same mistake as the writer, seeing the first instance is correct, they assume the rest is too.
This is what I love about PVS studio, it actually catches real life bugs like this one. They may look simple when shown in the spotlight but they do happen and can easily go unavoided. Instead of most other analyzers(or maybe I should call them linters) that only complain when you do something that obviously depends on UB like casting a pointer to an int, or missing paranthesis vs operator precedence.
Copy-paste errors are easier to detect if the code is lined up properly. The first example would have been fairly obvious if each if-then statement were put on one line instead of two, and then all the clauses would line up. The error would then have stood out.
I agree that the option (of passing the helper function no arguments at all) is not perfect either, but... (I also don't like that you need to call your stored errno's errnums or the like, but that's what we have.)