3 ms·
This test would be much more useful if they put all the errors into a broken string class, and asked you to find the bugs. Glancing at a broken class and seeing
by ekidd 15y ago
This test would be much more useful if they put all the errors into a broken string class, and asked you to find the bugs. Glancing at a broken class and seeing all the standard C++ bugs "pop out" is a useful skill for a C++ programmer. But instead of defining an actual, useful class with a bunch of bugs, they use horrendously nonsensical and broken code fragments. For example, I can't even figure out what the GrowArray function is supposed to do.
It's sort of like memorizing a chess board: A chess expert can memorize an actual game-in-progress quickly. But if you just place a bunch of random pieces around a chess board, a chess expert can't memorize it any better than anybody else.
The later examples with memchr are less painful, because memchr actually has well-specified behavior. But even there, the code (most likely) dereferences a pointer one past the end of an array, which is (1) invalid C++ and (2) not on their list of possible bugs. Granted, memchr is an internal compiler function, so it's allowed to rely on undefined behavior. But if this is a multithreaded system, that code potentially corrupts the heap's bookkeeping data structures while memchr is running.
Just looking at the first part of this test is like listening to somebody drag their fingernails across a blackboard. I'm sure that they're a lovely company, but ugh. To paraphrase Wolfgang Pauli, that code's so bad it's not even buggy—it's just semi-coherent nonsense.
Another interesting aspect of this test: Both the code examples and the bugs are pre-STL, pre-boost, and pre-TR1. For example, there's no question asking why you can't put a std::auto_ptr into an STL collection. And a huge fraction of the bugs in the test could be avoided by making rudimentary use of modern C++ APIs. Granted, some companies have legitimate reasons to avoid std::vector and std::tr1::shared_ptr. But avoiding those classes makes C++ vastly more error prone, which is usually a poor tradeoff in the modern world.
- deleted 15y ago[deleted]
- davisp 15y agoIts been awhile since I C++'ed heavily, but I think this isn't quite right: "But even there, the code (most likely) dereferences a pointer one past the end of an array, which is (1) invalid C++ and (2) not on their list of possible bugs." There's a weirdo thing with array allocation in C++ at least which makes it valid to use arrays with all of the std:: algorithms that take a start and end parameter. Unfortunately I can't remember it well enough to find a good reference but as I recall it was more than just a pointer comparison as the std:: algorithms wanted to be able to dereference it occasionally. Then again its been about four years since I've touched C++ so some of these details have leaked out and gotten mixed up.
- ekidd 15y agoThere's a weirdo thing with array allocation in C++ at least which makes it valid to use arrays with all of the std:: algorithms that take a start and end parameter. The actual rule is pretty subtle, and I don't have a copy of the C++ standard in front of me. But let's see if I can remember the precise rule. Given an array arr with length len, you're allowed to create the pointer arr+len. This is the end pointer you see everywhere in modern C++ APIs. However, if you try to deference arr+len, or try to create the pointer arr+len+1, the result is undefined. Typically, the former might point to a locked page, and the latter might overflow the pointer size. But if your compiler applies very aggressive optimizations, even stranger stuff can happen when you rely on undefined behavior: http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html http://blog.llvm.org/2011/05/what-every-c-programmer-should-... In the memchr example, the code potentially writes to arr+len, which has a decent chance of temporarily corrupting the implementation-dependent internal headers of the next heap block. In the presence of threads, this would very occasionally corrupt the internal state of new and delete, and debugging it would be a stupendously painful experience.