Feed aggregator
CodeSOD: Expressing a Leak
We previously discussed some whitespacing choices in a C++ codebase. Tim promised that there were more WTFs lurking in there, and has delivered one.
Let's start with this class constructor:
QBatch_arithExpr::QBatch_arithExpr(QBatch_unOp, const QBatch_snippet &, const QBatch_snippet &);You'll notice that this takes a parameter of type QBatch_unOp. What is that type? Well, it's an enumerated type describing the kind of operation this arithExpr represents. That is to say, they're not using real inheritance, but instead switching on the QBatch_unOp value to decide which code branch to execute- hand-made, home-grown artisanal inheritance. And while there are legitimate reasons to avoid inheritance, this is a clear case of "is-a" relationships, and it would allow compile-time checking of how you combine your types.
Tim also points out the use of the "repugnant west const", which is maybe a strong way to word it, but definitely using only "east const" makes it a lot easier to understand what the const operator does. It's worth noting that in this example, the second parameters is a const reference (not a reference to a const value).
Now, they are using inheritance, just not in that specific case:
class QBatch_paramExpr : public QBatch_snippet {...};There's nothing particularly wrong with this, but we're going to use this parameter expression in a moment.
QBatch_arithExpr* Foo(QBatch_snippet *expr) { // snip QBatch_arithExpr *derefExpr = new QBatch_arithExpr(enum_tag1, *(new QBatch_paramExpr(paramId))); assert(derefExpr); return new QBatch_arithExpr(enum_tag2, *expr, *derefExpr); }Honestly, in C++ code, seeing a pile of "*" operators and raw pointers is a sign that something's gone wrong, and this is no exception.
Let's start with calling the QBatch_arithExpr constructor- we pass it *(new QBatch_paramExpr(paramId)), which is a multilayered "oof". First, the new operator will heap allocate and construct an object, and return a pointer to that object. We then dereference that pointer, and pass the value as a reference to the constructor. This is an automatic memory leak; because we never trap the pointer, we never have the opportunity to release that memory. Remember kids, in C/C++ you need clear ownership semantics and someone needs to be responsible for deallocating all of the allocated memory- every new needs a delete, in this case.
Now, new QBatch_arithExpr(...) will also return a pointer, which we put in derefExpr. We then assert on that pointer, confirming that it isn't null. Which… it can't be. A constructor may fail and throw an exception, but you'll never get a null (now, I'm sure a sufficiently motivated programmer can mix nothrow and -fno-exceptions to get constructors to return null, but that's not happening here, and shouldn't happen anywhere).
Then we dereference that pointer and pass it to QBatch_arithExpr- creating another memory leak. Two memory leaks in three lines of code, where one line is an assert, is fairly impressive.
Elsewhere in the code, shared_pointer objects are used, wit their names aliased to readable types, aka QBatch_arithExpr::Ptr, and if that pattern were followed here, the memory leaks would go away.
As Tim puts it: "Some folks never quite escaped their Java background," and in this case, I think it shows. Objects are allocated with new, but never deleted, as if there's some magical garbage collector which is going to find the unused objects and free them.
[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!OpenAI Pushes AI Agent Capabilities With New Developer API
Read more of this story at Slashdot.
Geothermal Could Power Nearly All New Data Centers Through 2030
Read more of this story at Slashdot.
Team Behind Las Vegas Sphere Plans 5,000-Capacity 'Mini-Spheres'
Read more of this story at Slashdot.
AMD Ryzen 9 9950X3D With 3D V-Cache Impresses In Launch Day Testing
Read more of this story at Slashdot.
Microsoft is Replacing Remote Desktop With Its New Windows App
Read more of this story at Slashdot.
Preprint Sites bioRxiv and medRxiv Launch New Era of Independence
Read more of this story at Slashdot.
Zoox Robotaxis Do Not Meet Federal Safety Standards, Agency Says
Read more of this story at Slashdot.
Thousands of TP-Link Routers Have Been Infected By a Botnet To Spread Malware
Read more of this story at Slashdot.
Spain To Impose Massive Fines For Not Labeling AI-Generated Content
Read more of this story at Slashdot.
Vodafone Tells Employees To Follow RTO Policy Or Lose Bonuses
Read more of this story at Slashdot.
Southwest Airlines To End Free Checked Bags Policy For First Time in Its 54-Year History
Read more of this story at Slashdot.
The Surprising Impact of QR Code Menus on Diminishing Customer Loyalty
Read more of this story at Slashdot.
Why Extracting Data from PDFs Remains a Nightmare for Data Experts
Read more of this story at Slashdot.
Only Seven Countries Worldwide Meet WHO Dirty Air Guidelines, Study Shows
Read more of this story at Slashdot.
Half-Past Four Is the New Five O'Clock in More Efficient Workday
Read more of this story at Slashdot.
Firefox Certificate Expiration Threatens Add-ons, Streaming on March 14
Read more of this story at Slashdot.
California Pension Fund Labels Chevron and Saudi Aramco as Climate Investments
Read more of this story at Slashdot.
SpaceX Readies Starlink India Launch
Read more of this story at Slashdot.
Facebook Was 'Hand In Glove' With China
Read more of this story at Slashdot.