chapter7.html

Writing Bug-Free C Code

 another platform, changing the memory model of the program or
 upgrading to a new revision of the compiler.
 <br><br>
For whatever reason, your environment has changed.  When you
 recompile your program, you run the risk of missing warning
 messages.  This is because the behavior of the statement in which
 you are using the type cast may have changed, but the type cast
 masks the behavior change.
 <br><br>
<table bgcolor="#F0F0F0"><tr><td>
<img src="images/windows.gif">
There is another situation that is especially true in mixed-model
 programming under Windows.  To make matters worse, Microsoft's own
 sample code is a bad example because it is littered with totally
 unnecessary type casts.  Consider the following code fragment.
 <br><br>
<table bgcolor="#CCCCEE" cellpadding=0 cellspacing=0><tr><td><pre>
<b>An example of bad type casts</b>
MSG msg;
while (GetMessage((LPMSG)&msg, NULL, 0, 0)) {
    TranslateMessage((LPMSG)&msg);
    DispatchMessage((LPMSG)&msg);
    }
</pre></tr></td></table>
 <br>
Microsoft Windows programmers recognize this code as the main message
 loop for an application.  All messages that are placed in an
 application's message queue are dispatched by this message loop.
 <br><br>
The problem with this code is that all three type casts to LPMSG are
 totally unnecessary.  The code works great without the type casts. 
 The prototypes for GetMessage(), TranslateMessage() and
 DispatchMessage() all indicate that they take LPMSG as an argument. 
 The data type of &msg is PMSG due to the mixed-model environment.  I
 can only suppose that the programmer thought that PMSG must be type
 cast into what the functions expected, an LPMSG.  This is simply not
 the case.  In mixed-model programming, the compiler promotes a near
 pointer to the object to a far pointer to the object in all cases
 that a far pointer to the object is expected.  In other words, the
 compiler is implicitly performing the type cast for you.
</td></tr></table>
<br>
<table bgcolor="#F0F0F0"><tr><td>
<img src="images/windows.gif"> <b>7.13.1 Mixed-Model Programming Implicit Type Cast Warning</b>
 <br><br>
In mixed-model programming there exists a subtle problem if you write
 code that allows NULL to be passed through as one of the argument
 pointers.  Consider the following code.
 <br><br>
<table bgcolor="#CCCCEE" cellpadding=0 cellspacing=0><tr><td><pre>
<b>GetCpuSpeed(), demonstrating implicit type cast problem</b>
int APIENTRY GetCpuSpeed( LPSTR lpBuffer )
{
    int nSpeed=(calculation);
    if (lpBuffer) {
        (fill buffer with text description of speed)
        }
    return (nSpeed);

} /* GetCpuSpeed */
</pre></tr></td></table>
 <br>
GetCpuSpeed() always returns the CPU speed as an int, but as an
 option it also creates a text description of the CPU speed in the
 provided buffer if the buffer pointer is non-NULL.  Now what happens
 when you call GetCpuSpeed()?
 <br><br>
<table bgcolor="#CCCCEE" cellpadding=0 cellspacing=0><tr><td><pre>
<b>Calling GetCpuSpeed()</b>
PSTR pBuffer=NULL;
int nSpeed1=GetCpuSpeed(NULL);
int nSpeed2=GetCpuSpeed(pBuffer);
</pre></tr></td></table>
 <br>
In both cases you want only the integer speed and not the text
 description.  In the first case, GetCpuSpeed(NULL) behaves as
 expected.  However, in the second case, GetCpuSpeed(pBuffer) fills
 in a text buffer.  The problem is that pBuffer is a near pointer and
 that GetCpuSpeed() expects a far pointer.  No matter what value is
 contained in pBuffer, it is considered a valid pointer and the type
 cast of a near pointer to a far pointer (implicitly by the compiler
 or explicitly by you) uses the data segment value as the segment for
 the far pointer.
 <br><br>
In other words, when the pBuffer near pointer is converted to a far
 pointer, the offset is NULL, but the segment value is non-NULL.
 <br><br>
From experience, I have found that correctly writing code in
 situations like this is too problematic.  My solution to this
 problem has been to move away from mixed-model pointers and stick
 with far pointers.
</td></tr></table>


<br><a name="usesizeof"><br></a>
<big><b>7.14 Use sizeof() on Variables, not Types</b></big>
 <br><br>
How do you use sizeof() in your code?  Do you typically use sizeof()
 with a variable name or a data type?  While at first glance the
 distinction may not seem to matter that much, at a deeper level it
 matters a lot.  Consider the following example.
 <br><br>
<table bgcolor="#CCCCEE" cellpadding=0 cellspacing=0><tr><td><pre>
<b>Using sizeof(), a bad example</b>
int nVar;
...
DumpHex( &nVar, sizeof(int) );
</pre></tr></td></table>
 <br>
DumpHex() is a general purpose routine that will dump out an
 arbitrary byte range of memory.  The first argument is a pointer to
 a block of memory and the second argument is the number of bytes to
 dump.
 <br><br>
Can you spot a possible problem in this example?  The sizeof() in
 this example is operating on the int data type and not on the
 variable nVar.  What if nVar needs to be changed in the future to a
 long data type?  Well, sizeof(int) would have to be changed to
 sizeof(long).  A better way to use sizeof() is as follows.
 <br><br>
<table bgcolor="#CCCCEE" cellpadding=0 cellspacing=0><tr><td><pre>
<b>Using sizeof(), a good example</b>
int nVar;
...
DumpHex( &nVar, sizeof(nVar) );
</pre></tr></td></table>
 <br>
In this new example, sizeof() now operates on nVar.  This allows
 DumpHex() to work correctly no matter what the data type of nVar is.
  If the type of nVar changes, we will not have to hunt down in the
 code where the old data type was explicitly used.


<br><a name="avoiddeepnest"><br></a>
<big><b>7.15 Avoid Deeply Nested Blocks</b></big>
 <br><br>
There are times when, for any number of reasons, you end up writing
 code that is deeply nested.  Consider the following example.
 <br><br>
<table bgcolor="#CCCCEE" cellpadding=0 cellspacing=0><tr><td><pre>
<b>Deeply nested code</b>
void DeepNestFunction( void )
{
    if (test1) {
        (more code)
        if (test2) {
            (more code)
            if (test3) {
                (more code)
                if (test4) {
                    (more code)
                    }
                }
            }
        }

} /* DeepNestFunction */
</pre></tr></td></table>
 <br>
While this nesting is only four deep, I've had times when it would
 have gone ten deep.  When nesting gets too deep, the code becomes
 harder to read and understand.  There are two basic solutions to
 this problem.
 <br><br>
Unroll the tests.  The first solution is to create a boolean variable
 that maintains the current success or failure status and to
 constantly retest it as follows.
 <br><br>
<table bgcolor="#CCCCEE" cellpadding=0 cellspacing=0><tr><td><pre>
<b>Unrolling the deep nesting</b>
void UnrollingDeepNesting( void )
{
    BOOL bVar=(test1);
    if (bVar) {
        (more code)
        bVar = (test2);
        }
    if (bVar) {
        (more code)
        bVar = (test3);
        }
    ...

} /* UnrollingDeepNesting */
</pre></tr></td></table>
 <br>
Call another function.  The second solution is to package the
 innermost tests into another function and to call that function
 instead of performing the tests directly.


<br><a name="smallfunctions"><br></a>
<big><b>7.16 Keep Functions Small</b></big>
 <br><br>
The primary reason to keep functions small is that it helps you
 manage and understand a programming problem better.  If you have to
 go back to the code a year later to modify it, you may have
 forgotten the small details and have to relearn how the code works. 
 It sure helps if functions are small.
 <br><br>
As a general rule, try to keep functions manageable by restricting
 their length to one page.  Most of the time functions are smaller
 than a page and sometimes they are a page or two.  Having a function
 that spans five pages is unacceptable.
   <blockquote><table bgcolor="#E0E0E0" border=1 cellpadding=2 cellspacing=0><tr><td>
   As a general rule, try to keep functions under one page.
   </tr></td></table></blockquote>
If a function starts to get too large, step back a moment and try to
 break the function into smaller functions.  The functions should
 make sense on their own.  Remember to treat a function as a method
 that transitions an object from one valid state to another valid
 state.  Try to come up with well-defined, discrete actions and write
 functions that perform these actions.


<br><a name="releasedebug"><br></a>
<big><b>7.17 Releasing Debugging Code in the Product</b></big>
 <br><br>
Is there such a thing as a debug build and a retail build of your
 product?  Should there be?  No, I do not think so!  Let me explain
 why.  I believe that any released application should be able to be
 thrown into debug mode on the fly at any time.
 <br><br>
In the applications that I develop, I have a global boolean variable
 called bDebugging that is either FALSE or TRUE.  I place what I
 consider to be true debugging code within an if statement that
 checks bDebugging.  This is usually done for debugging code that
 adds a lot of execution overhead.  For debug code that does not add
 much overhead, I just include the code and do not bother with
 bDebugging.
 <br><br>
The benefit of doing this is that there is only one build of your
 product.  That way, if a customer is running into a severe problem
 with your product, you can instruct the customer how to run your
 product in debug mode and quite possibly find the problem quickly.
 <br><br>
I do not consider WinAssert() and VERIFY() to be debugging code.  In
 the programs that I write, WinAssert() and VERIFY() are not
 switchable by bDebugging.  Instead, they are always on.  The
 reasoning is simple.  Would you like to know a bug's filename and
 line number in your program or would you just like to know that your
 program crashed somewhere, location unknown?
   <blockquote><table bgcolor="#E0E0E0" border=1 cellpadding=2 cellspacing=0><tr><td>
   WinAssert() and VERIFY() are not debugging code.
   </tr></td></table></blockquote>
If you object to the users of your product seeing assertion failures
 and run-time object verification failures, I recommend that you
 instead silently record the error in a log file.  By doing this, you
 will have some record of what went wrong in the program when the
 customer calls you.
 <br><br>
In a program that ships with WinAssert() and VERIFY() on, the program
 alerts the user to the exact filename and line number of a problem. 
 If the fault-tolerant syntax is used, the program continues to run. 
 Oftentimes, just knowing that a program failed at this one spot is
 enough to scrutinize that section of the code and find the problem.
 <br><br>
It is important that the fault-tolerant forms of WinAssert() and
 VERIFY() be used.  Doing so ensures that the program continues to
 run after a fault.
   <blockquote><table bgcolor="#E0E0E0" border=1 cellpadding=2 cellspacing=0><tr><td>
   The fault-tolerant forms of WinAssert() and VERIFY() should always be
   used.
   </tr></td></table></blockquote>
Sometimes a filename and line number are not enough to track down a
 problem.  At times like these, a stack trace is highly desirable.


<br><a name="stacktrace"><br></a>
<big><b>7.18 Stack Trace Support</b></big>
 <br><br>
A key debugging tool that I use for tracking down problems in my code
 is utilizing stack trace dumps from a fault.  Sometimes only knowing
 the filename and line number of a fault is not enough to track down
 a problem.  In these cases, the call history that led up to the
 problem is often enough.
 <br><br>
For example, I once had a program that was faulting at a specific