The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

Rules of thumb for robust C++ software

Our company doesn't have software quality guidelines (we code in C++), and wrestling with weird bugs and crashes is sucking all joy out of our lives. Researching deeply into improving  software robustness is a fairly long term endeavour, and I was thinking maybe coming up with a few simple "rules of thumb" that provide the greatest bang for the buck in reducing crashes and memory leaks due to mongrel programming errors might be a good first step.

The following are a few I can think of right off the bat (I'm very much a newbie mind you!):

1. Use dynamic memory allocation only where they're *absolutely* necessary.

2. The owner of a dynamically allocated object should be clearly defined at all times. Once deallocated, set the pointer to zero so that any immediate use will cause hard crashes.

3. For all functions:
  (a) Document what the function does
  (b) Document what the legal parameters are, and if not documented, check for the legality of parameters within your code or ensure that illegal parameters will not lead to catastrophies
  (c) Clarify preconditions that must be satisfied if the call is to succeed, and check for the preconditions within the function.
  (d) Document how the function may fail (if applicable), and how the failure will be reported
 
4. Always handle failures of functions that you call if the following portions of your code assumes their success.

5. Use vectors and other STL classes instead of arrays, sprintfs, etc.

What would be a few such rules in your opinion?
Sam
Saturday, November 19, 2005
 
 
You might want to look at "C++ Coding Standards" by Sutter and Alexandrescu. 101 rules in sections. Each section has a favorite rule. Start with a small subset and work up. Like the Joel test :-)
expr
Saturday, November 19, 2005
 
 
1b. Avoid malloc and free in C++ programming.
http://www.codeproject.com/tips/newandmalloc.asp
http://oopweb.com/CPP/Documents/CPPHOWTO/Volume/C++Programming-HOWTO-9.html

4b. ALWAYS handle failures of functions that you call, even  if the following portions of your code do not assume their success, and be it only for an entry in the log file. Else why are you calling the functions at all?
Secure
Saturday, November 19, 2005
 
 
> I was thinking maybe coming up with a few simple "rules of thumb"

Rules are easy. Getting people to follow them is the hard part. That's why C++ is such a difficult language. Even though you can be safe, most people won't be.

> Use dynamic memory allocation only where they're *absolutely* necessary.

Not important once you solve your later points.

> The owner of a dynamically allocated

Use smart pointers.

> Document what the function does

You are an optimist.

> Always handle failures of functions

Use exceptions.

> Use vectors and other STL classes instead of arrays, sprintfs, etc.

Yuck for STL, but better than raw buffers.
son of parnas
Saturday, November 19, 2005
 
 
Hey expr, think you could do a fellow developer a small favour by listing 3 of your favorite rules? I'd  really appreciate it a lot! :-)

I've been wanting to buy the coding standards book for some time now, and will do so at the very next opportunity. The most serious problems we're having are not really high-level design related (though God knows we've enough of those..), but are more due to engineers doing extremely shabby coding.
Sam
Saturday, November 19, 2005
 
 
> but are more due to engineers doing extremely shabby coding.

And that's the problem rules can't solve.
son of parnas
Saturday, November 19, 2005
 
 
1) Don't scatter deletes all over the place. Use either RAII or shared/smart pointers to avoid dangling pointer dereferencing.

2) Asserts, lots of them.

3) Use as few communication points between different threads as possible.
Captain Obvious
Saturday, November 19, 2005
 
 
In the 15 years I have been coding C++, IMO the biggest problem is getting idiots to follow any kind of rules. You can insist that people use smart pointers, RAII, etc., but in the end you have to have enforcement.

So,

1. Make the rules.
2. Have code reviews.
3. Give everyone 2-3 changes to get it together.
4. Be ready to fire the miscreants, no matter their status or position.

I used to work with a guy who stored file handles and pointers as global variables and never closed the files or freed memory because he said the OS would take care of it.

The guy should have been shown the door, but he was senior level and the boss liked him. His code sucked.
MBJ Send private email
Saturday, November 19, 2005
 
 
I did not offer any examples because:
1. We do not have a written standard
2. I probably break a lot of standard rules
some good things:
a. Compile cleanly at high warning levels (on several compilers -- differences between gcc and VC++)
b. Use meaningful (and truthful) variable and function names.
c. Try not to nest too deeply (if within for within while etc.)
d. Try to keep your functions short.
global:
If you are going to have a standard, write it down and give it to all affected parties. ( I asked a client if they had a coding standard, they said no then complained when I violated their implicit standard which I was supposed to guess by looking at their code.)
expr
Saturday, November 19, 2005
 
 
We do mission-critical, real-time software in C++ and simply can't have the system crashing either (and we don't, although we certainly have our share of other bugs). We have two main architectural rules and coding rules that stem from that. Architecturally:

1) The biggest -- don't have objects creating other objects (i.e. don't use 'new!') except in only one or two class factories. The application "glue" or main module  creates all instances of the objects needed and then initializes them by passing them pointers to the other objects they depend on. When you have dynamic objects like data objects, you keep all their 'newing' in one place (like the loader) and it is also responsible for deleting them when told to shutdown. 

2) Use the Initialize and Shutdown metaphor throughout instead of relying on a constructor and destructor. In the constructor you initialize all member pointers to NULL, and at the beginning of every function you always check if any of the pointers used in that function are NULL. If so,  it's probably getting called before initialization or after shutdown, which will always happen especially as the project evolves and things are moved around. So you just always handle it. On Shutdown all member pointers are set to null, and whoever's calling everybody's Shutdown (like the core code) only at the end does it deletes the objects or tell the one or two classfactories to shutdown.

These lead to a handful of coding rules that become instinctive to people after a short time:

1) Any time you add a member pointer you automatically go to the constructor and initialize it to NULL (if you're not using smart pointers), then go to the shutdown method and set it to NULL.

2) At the beginning of every function you always check all the pointers used in that function for NULL (if they should never be), or before calling any object if you're not using smart pointers you always check it for NULL (if sometimes they may be). You just never assume things are valid because functions end up getting moved around down the line.

3) Any time you find yourself having to use 'new' it should be an automatic special-case red flag that makes you double-check you're deleting it before exiting.

It's worked for us over the years, anyway.
Bill
Saturday, November 19, 2005
 
 
Consider using a garbage collector.

Yes, in C++.  Google for "boehm garbage collector".

There are a few quirks, relating to interfacing any GC to C++, but once you do this, a lot of your pointer issues are simplified.

There are a lot of models that you can't easily implement with a "one and only one owner who is responsible for freeing" methodology.
David Jones Send private email
Saturday, November 19, 2005
 
 
why would you ever move construction/destruction out of the constructor/destructor.  You do realize that they are called automatically as the object enters and leaves scope?

new and delete aren't bad, they just need to be kept from the user. (by user I mean user of the functionality, in any given product we are both the user and the library writer, know which role you are in any given section of code)  std::string for example uses new and delete, but it wraps them up so that the user doesn't have to worry.  Folks using char* are the ones having bugs.
Tom Cahalan Send private email
Saturday, November 19, 2005
 
 
> The biggest -- don't have objects creating other objects

Use smart pointers and you can get rid of this rule, Having factories create objects is odd diven it's deletion that is the problem.

> Use the Initialize and Shutdown metaphor throughout instead of relying on a constructor and destructor.

You never say why. RAII is much much safer.
son of parnas
Saturday, November 19, 2005
 
 
Reach "Effective C++" by Meyer.
Use lots of asserts.
Unit test where appropriate.
Andy Brice Send private email
Saturday, November 19, 2005
 
 
> Reach "Effective C++" by Meyer.

Sorry, that should have been:

 Read "Effective C++" by Meyers.
Andy Brice Send private email
Saturday, November 19, 2005
 
 
My most important rule: Use RAII/smart pointers/scope guards religiously. There must be a VERY good reason for not using them.
sloop
Saturday, November 19, 2005
 
 
" why would you ever move construction/destruction out of the constructor/destructor."

You don't, pointers are initialized in the constructor (to NULL) and set to what they are supposed to point to in Initialize.

" > The biggest -- don't have objects creating other objects

Use smart pointers and you can get rid of this rule, Having factories create objects is odd diven it's deletion that is the problem."

The problem is smart pointers don't know the object it's pointing to has been deleted (unless you get into awkward reference counting which experience with COM shows people often circumvent), so you're still stuck with access violations.
 
"> Use the Initialize and Shutdown metaphor throughout instead of relying on a constructor and destructor.

You never say why. RAII is much much safer."

Exceptions don't work across multiple processes, or client/server and distributed systems, and also with remote procedure calls and multithreading you can end up with pending calls still coming in after an object has been deleted. "Shutting down" everything first ensures the calls complete (since ShutDown() doesn't return until worker threads are stopped), and only then do you delete.

Like I said, this has always worked for us although maybe its benefit is unique to a distributed and multithreaded system. RAII is nice for single processes, but it wouldn't work for us.
Bill
Saturday, November 19, 2005
 
 
"The problem is smart pointers don't know the object it's pointing to has been deleted (unless you get into awkward reference counting which experience with COM shows people often circumvent), so you're still stuck with access violations."

How is it awkward?  Instead of the user manually tracking the reference count you leave it up to the smart pointer.  The smart pointer has two members, the first is the pointer to the object, the second is the pointer to the reference count.  The constructors (including copy constructor), assignment operator, and destructor manage the reference count for you.  The smart pointer type would be a template of course, and so once you write it (or download a library) you never need to worry about it again.

Heck here's one now, already made for you and tested by a multitude of users:
http://www.boost.org/libs/smart_ptr/shared_ptr.htm
it will likely be included in the next version of the standard.
Tom Cahalan Send private email
Saturday, November 19, 2005
 
 
It's awkward because in the real world, clients crash, connections are lost, and references end up not being released anyway.
Bill
Saturday, November 19, 2005
 
 
> Exceptions don't work across multiple processes

They work at the layer that makes a request and forms a response. That's certainly sufficient.

> up with pending calls still coming in after an
>object has been deleted

I don't see how. Deregister before deletion and the pending calls could never be dispatched.

> "Shutting down" everything first ensures the calls
>complete (since ShutDown() doesn't return until
>worker threads are stopped), and only then do you delete.

If you are talking about shutting down active objects then I agree. But that should be a rather small subset of objects in your system.

>benefit is unique to a distributed and multithreaded system.

You would be wrong in thinking you are the only one with this experience.

>RAII is nice for single processes, but it wouldn't work for us.

Me thinks you have an odd architecture where threads are being created and delete all the time.

Typically you would have threas the form a pipeline or an active object that acts as an end point for messages. The only time these should ever be deleted is on a clean system shutdown which should be quite rare.
son of parnas
Saturday, November 19, 2005
 
 
> It's awkward because in the real world, clients crash, connections are lost, and references end up not being released anyway.

As you can't hold a reference to remote memory non of these things would cause leakage.
son of parnas
Saturday, November 19, 2005
 
 
"As you can't hold a reference to remote memory non of these things would cause leakage."

No, with DCOM for example you can in fact hold a reference to "remote memory." Users in the real world will press the power button to shut down, and the server gets stuck with unreleased references. References just don't work...
Bill
Saturday, November 19, 2005
 
 
DCOM references do time out after 6 minutes. But DCOM sucks in lots of other ways, so I'll just take your word for it.

In general, for reliable C++ you need to do a couple of things:

1) C++ does have an implicit distinction between reference types and value types. Decide up front what one you're writing. Will this type be created via new or on the stack? Will it be typically passed by reference/pointer or passed by value? If pass by reference, explicitly disable the copy constructor and assignment operators. If pass by value, explicitly implement a copy constructor, assignment operator, and swap function (so you can implement assignment in terms of copy in an exception safe way).

2) Prefer pass-by-value semantics for any objects that wrap OS resources (Sockets, file handles, GDI object, etc.)

3) Use exception handling unless you have a VERY good reason not to. "It's too slow" is not a good enough reason unless you've got a profile that proves it. "My compiler doesn't support it" only works if you can't get another compiler.

4) Use RAII (Resource acquisition is initialization) whenever you acquire resources from the OS that you need to free later.

5) Work out in explicit detail when you have shared resources. The previous poster is correct; reference counting is often very hard to get right, and sometimes (such as circular references) doesn't work at all.

6) Try to avoid shared resources. ;-)

Good luck!

-Chris
Chris Tavares Send private email
Sunday, November 20, 2005
 
 
> with DCOM for example you can in fact hold a reference to "remote memory."

You realize of couse that dcom is just a made up library and you are not holding a reference to real memory?
son of parnas
Sunday, November 20, 2005
 
 
+1 on Read "Effective C++" by Meyers.
Also Read "More Effective C++" by Meyers.

If you get interested in some of the 'heavy detail' you might check out 'Imperfect C++' which digs in depth about certain frailties of the language and suggestions for working around them.

Often I have seen the design of C++ program to be based around what it needs to do now enough to get it to work.
People start by doing good RAII and all seems to be working ok in the first cut.  They then come back later, and find that they want to move an object to a list of objects or other such things, and they forget that they didn't fully define their classes in the first place, they didn't create the copy/assignment constructors and the default ones don't work right.

Another from Myer (in my words) is to throw by allocator and catch by reference.  Don't throw/catch  pointers, it's next to impossible to decide who should delete it.

Use multiple compilers, mostly they don't fully meet the current standard, and there can be ambiguities.  Each compiler can show up a different subset of problems, some will also offer interesting effects, such as multi threading issues showing up better under the Intel compiler.

Sunday, November 20, 2005
 
 
My favourite top three for C++

1) Object Oriented principles like Law of Demeter, Design by Contract and so on. Because fragile software in C++ comes almost always from bad design.

2) Select a subset of the language and deviate only from that subset when you absolutly need to. Also it gives people a good chance to learn that subset well and expand on that when needed.

3) Have your coding guidelines in an internal wiki where everyone in the team can add and improve your coding process. If they run into a monstrous bug they can analyse what went wrong and devise a procedure/guideline how to avoid it, adding it in the wiki.
Rediscovering C++
Sunday, November 20, 2005
 
 
Everything I've seen mentioned so far is about code writing practices.

Those things are definitely vital, but it is always possible to run into problems even if you're trying to follow best practices.

This is why in addition to all the kinds of good coding practices mentioned before, you also need to have diagnostics always running on your debug build. This means having a system that automatically checks for and reports on any leaked memory, heap corruption, stack corruption, or uninitialized memory usage on every run of a debug build.

Keep the build clean, don't let checkins come in that cause asserts or memory leaks during regular usage. This way it is easier to track down the cause of a reported leak or heap corruption, since it will be related to the stuff being changed, intead of possibly anywhere in the product.

Having this kind of a system set up, and a good regimen of testing the product before checking in your changes, is very important to improving quality.
Michael G
Monday, November 21, 2005
 
 
Here's a rule for exceptions: try and preserve as much information as possible across language boundaries. In general, anywhere where you're about to exit the C runtime (RPC call boundaries, thread functions, main function, exiting to another language) you need to catch and handle/translate exceptions.

For instance, for RPC calls, this means that you automatically serialize exceptions (with any nested exceptions, if your exception classes support that concept) and deserialize + rethrow on the other side. Then the function on the RPC server can throw an exception and everything will be handled properly.

Another instance: for COM wrappers, you should catch exceptions and translate them to COM exceptions via the IError interface.

It drives me bonkers to see people chucking away exceptions at system boundaries because they can't be bothered to decide what to do with them. Eg

int dll_function()
{
  try()
  {
      whatever();
  }
  catch(...)
  {
      return -1;
  }
}

Ugh!
Matt Morris
Tuesday, November 22, 2005
 
 
Simple solution: do code reviews.  They don't have to be formal.  At my last job, we just emailed diffs to two other developers, even for one-line changes.  Worked smoothly.  As mentioned, all the rules won't help any if people ignore them.  If they know their code will be scrutinized by others, they will tend to be more careful.
Chris
Tuesday, November 22, 2005
 
 
Use a static code analysis tool like PREFast

http://www.microsoft.com/whdc/devtools/tools/PREfast.mspx
SeanH
Wednesday, November 23, 2005
 
 
>Use a static code analysis tool like PREFast

Or Gimpel PC-lint, if you aren't writing drivers.
Andy Brice Send private email
Wednesday, November 23, 2005
 
 
There's times when exceptions should be thrown away.  If someone's writing a DLL with a plain C interface and a C++ implementation, they can't throw exceptions.  The advantage to this is that the DLL can be used regardless of the language and compiler the user is using.  Of course, it should still be a separate error code for each exception, although a generic 'something didn't work' is important as you can never be certain of every exception that will be thrown if you use third party code.


Btw, people, the poor guy here admitted to being a C++ newbie - anyone who's afraid of allocating memory in a single-threaded non-distributed application simply is not ready to deal with DLLs, threading, or any form of distributed processing.  Let him learn how to program before demanding he learns about the more advanced topics, ok?


On the other hand, I'm a bit worried about all the "I'm in charge of a team, teach me how to program" posts that keep appearing. Do people really get senior developer positions when they've barely read half way through a 'Learn X in 21 days' book?  Why is a company writing software in C++ if they haven't hired someone who's competent in that language?  Sure, the smart guy who knows Java can join the team and pick up C++ quickly - but that's different to expecting the entire team to hit the ground running with no training time, and the presence of mentors makes the process a whole lot more efficient for the company.

Really, the actual rules are (0a) Actually learn to program in your language of choice, and understand that this means months of work, and longer if you don't have a solid background in a sufficiently similar language and (0b) read books from experts, because otherwise you'll just set off an argument that you're not yet qualified to judge.  When people argue back and forth about the merits of RAII or exceptions vs return codes or anything else, the newbies by definition don't yet know enough to actually work out which side is right, or to understand if there's tradeoffs not being mentioned in the argument they get to witness.

Wednesday, November 23, 2005
 
 
> Why is a company writing software in C++ if they haven't hired someone who's competent in that language?

Manager: What language do we use here at Monster Corp?
Chorus: C++
Manager: Get me some of them, cheap.
son of parnas
Thursday, November 24, 2005
 
 
Why don't they have managers that know he asks...

Because, like programmers, managers should have sufficient experiance to be moved all about the place, if you move to a project using C++ or SAS or Notes or... doesn't matter, you just learn it.

Yeah.
Right.

Thursday, November 24, 2005
 
 
Obviously a DLL interface can't chuck exceptions - but that doesn't mean that you *have* to lose exception information if you have a DLL interface.

You can - and should - provide a "get last exception information" function so people can write a layer on top of the DLL interface that propagates exceptions correctly.

Of course, this means that people do have to write a wrapper around the DLL interface that checks for error codes, gets exceptions, and so on. But at least they now have the opportunity to do so.

To see an example, check out the JNI: http://java.sun.com/docs/books/jni/html/exceptions.html

If you don't provide that then you're dooming people to lose error information at the boundary defined by the DLL interface. That's something that I, personally, try to avoid.
Matt Morris
Friday, November 25, 2005
 
 
Personally I have found having a test case for every object invaluable.

Each time a change is made to a class, the test cases are run and verified.

Each time a bug is found, look at the test cases and see if a test for the crash case is missing, and if it is add it to the test script.

Yes, its a lot of effort but you will find your bugs come down as you are building on a solid foundation.
Paul Todd Send private email
Sunday, November 27, 2005
 
 
It's funny how much of C++ problem discussions center around memory handling (and grabbing/releasing system resources generally).  In the designs I've done, that probably ranks 3rd (or below) in severity behind:

1) (With a bullet) The typical problems that pop up in multithread/multiprocess systems (races, blocking, hardware failure on occasion)...most notably being the ones with a fair number of heterogeneous processors (DSPs + RISC + communication processors for example).

2) Problems with algorithms.  Precision/rounding/clipping issues, edge conditions, just flat mistakes in implementation.

Aside from just being careful in both design and implementation (standardized handling for synchronization, for example), there's not a lot of C++ trickery you can do to really drop the bug work load.

Heck, it would have been great to just deal with memory leaks and the like.
MrUnemployedEmbeddedGuy Send private email
Sunday, November 27, 2005
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz