Tuesday, October 13, 2009

Please Be Nice and Include Everyone

Well I saw something today that made my hair stand on end. One of my coworkers had done something he should have never, never, never, never....never, never (did I mention never) done. If you are not familiar with C you should know this. In C a functions' signature must be declared before it is used. That is you cannot call a function without first at least telling the compiler what it's interface looks like. This was a decision made in the early days of the language to make C easer to parse.

To this end C has two concepts Deceleration or saying something exists but not what it is and Definition which is saying what it is. You can't use anything, be it variable, function or whatever without at least saying it exists first. C also has two different file types the *.h file is reserved for the interface of a module and the *.c for the implementation. Which fits into the Deceleration/Definition scheme perfectly. Basically the *.h file is to be used for Decelerations only. It occasionally can contain some Definitions. To see a Definition in a *.h file usually signals a programming error however.

So ideally a C program should be structured so that there is one "module" per *.c file. Each *.c file has a corresponding *.h file with the module's public interface declared in it. Notice that I said "public interface" something that should be done is to take utility functions that have no bearing on the module's purpose out of the header file. You can define/declare them entirely in the *.c file as static. Or if they are more generally useful they can be moved to a sub-module that other modules can use and be declared/defined normally.

This is where the break down comes in. The developer had taken my advice but not really understood it. He had half way done both things. A function had been forward declared in the *.c file that used it but was defined in a separate *.c file. Which is a big no-no. The C compiler uses the deceleration in the *.h file to check and make sure that you are calling the function with the right number and types of parameters.

By forward declaring the function like he did the compiler had no chance what-so-ever of catching this programming error. While in the file where the function is used the compiler would take the forward deceleration as gospel and compile without even a warning. Then while compiling the file the function was in it would take the definition as gospel and compiled fine. However the generated code now is mismatched. If the two function signatures are ever different the compiler will happily call the functions with the wrong parameters. Another of C's many shotguns to blow your foot off.

This is the second time I've seen this in the company. The first time, was in a long existing code base, I was a little flabbergasted and spent a good bit of time cleaning it up. This time all I could do was shake my head and tell them to fix it. So the moral of the story is ...DON'T DO IT!!! Its a huge headache to find when it breaks and it will break. If you are putting a function in a *.c file and calling it from another *.c then make a header for the module. If the function is never used outside the current module then have it entirely inside the module. There is no other right way to do this!

No comments:

Post a Comment