Thread: Help understanding structuring multiple .c and .h files (intial linker process)

I have a var.h file where I create many global variables. I have many .c files where I need to access them (duh). I include var.h in almost all .c files. In the var.h file, I assume that I can simply use the following

Code:

#define EXT extern
#define VOL volatile
    //Used to flag timer events in main.c
EXT VOL FlagStatus  one_ms_flag, ten_ms_flag, twnetyFive_ms_flag, fifty_ms_flag, hundred_ms_flag, sec_flag;
    //Sequence timing events - incrimented every 1 ms in main.c
EXT VOL uint16_t    event_counter;

But when doing this I receive the "undefined reference to" error message. But when I use the code used by my co-workers...

Code:

#ifdef MAIN 
    #define EXT 
#else 
    #define EXT extern
#endif
#ifndef VOL
#define VOL volatile
#endif
    //Used to flag timer events in main.c
EXT VOL FlagStatus  one_ms_flag, ten_ms_flag, twnetyFive_ms_flag, fifty_ms_flag, hundred_ms_flag, sec_flag;
    //Sequence timing events - incrimented every 1 ms in main.c
EXT VOL uint16_t    event_counter;

Everything works great! I have no clue why. Can some one go back to the basics and explain why it works 1 way but not the other? I thought that both would work since variables can be declared as many times as we want but only defined once. Thanks for the tips.

First, I would like to say that I strongly discourage this whole approach of using tons of global variables and changing their linkage characteristics (and volatility!) via pre-processor stuff. There are times when that is the best/only solution, however those are few and far between, and there are very compelling reasons. I sincerely hope you guys have a really, really good reason for doing it this way. Can I ask what kind of system you're developing for and why you've taken this approach?

As for your question: You must understand the difference between declaration and definition. A declaration simply states the name of the variable and it's type (and possibly qualifiers like external linkage and volatile changes). It basically helps the compiler type-check the code, making sure you are using the right variables in the right ways. A definition actually creates space for the variable, to hold the value. The message you receive is because your version of the code only provided declarations and never a definition. Thus there was no actual variable, and when the linker came along and tried to fix all the references to those globals in all the files, it never actually found the variables, hence the references were undefined.

The extern keyword, if present, makes those lines mere declarations and not definitions, i.e. no actual variable. Notice your co-workers' code has some extra ifdef statements. Basically, if the pre-processor symbol MAIN is defined, they define EXT and VOL as empty strings, so those lines become actual definitions and not just declarations, thus allowing the linker to link the references to that variabe. Note, you should only #define MAIN in one .c file (I assume the one containing the main function). Otherwise you will have multiple copies of the variable and the linker wont know which one you want to use.

Originally Posted by anduril462

... this whole approach of using tons of global variables and changing their linkage characteristics (and volatility!) via pre-processor stuff.

I've worked at several companies where this method was commonly used for large projects. Another common corporate standard is to use static for any variable or function where the scope should be limited to just that source file.

I've worked at several companies where this method was commonly used for large projects.

O_o

The existence of infinitely many cases where poor design has been employed doesn't change the nature of poor design.

I've seen a lot of companies use `gets'; that doesn't make the function safe.

Another common corporate standard is to use static for any variable or function where the scope should be limited to just that source file.

Simply using a feature with concern appropriately to the nature of the feature isn't a problem.

If a variable shouldn't have external linkage `static' is the correct approach.

Defining a variable or a function as `static' only to throw that definition in a commonly used project header is entirely a different thing.

Soma

Originally Posted by rcgldr

I've worked at several companies where this method was commonly used for large projects. Another common corporate standard is to use static for any variable or function where the scope should be limited to just that source file.

The fact that several companies do something a particular way doesn't mean it's a good thing. There are lots of bad practices that I've come across at the various companies I've worked at. Some of them I didn't realize were not good ideas until I had many more years of programming experience under my belt.

The alternative you speak of is half-way to the solution I would propose:
Declare them at file scope in a .c file as you suggest, but also declare functions for controlling access to the variables and limiting how they can change. Define an interface to working with the variables so that you have better control over them. For example, if you have a counter variable like event_counter, I'm guessing you don't want some random piece of code to set it to some arbitrary value like:

Code:

event_counter = 42;

That seems like a bad thing to do to a counter. Instead, since it counts things, you should only have perhaps a get function, a reset function, an increment function, and if needed, an increment by N function. E.g.:

Code:

static uint16_t event_counter = 0;

void reset_event_counter(void)
{
    event_counter = 0;
}

void increment_event_counter(void)
{
    event_counter++;
}

void increment_event_counter_by_N(uint16_t N)
{
    event_counter += N;
}

uint16_t get_event_counter(void)
{
    return event_counter;
}

Make a .h file with extern prototypes for those functions, include as needed and compile/link the .c file where needed. Now nobody can misuse your event counter variable by setting it to some arbitrary value.

Now nobody can misuse your event counter variable by setting it to some arbitrary value.

O_o

You've only decorated the issue.

If `event_counter = 42;' is a problem, the solution isn't decoration, and for example, being unable to trace variable dependencies generally is a problem.

You can throw a "Singleton" at the variable, but you'd still have the same problems.

*shrug*

If you want to fix problems, such as dependance tracking, related to global variables you can't just decorate the same globals.

Soma

Code:

reset_event_counter();
increment_event_counter_by_N(42);

Originally Posted by phantomotap

O_o

You've only decorated the issue.

If `event_counter = 42;' is a problem, the solution isn't decoration, and for example, being unable to trace variable dependencies generally is a problem.

You can throw a "Singleton" at the variable, but you'd still have the same problems.

*shrug*

If you want to fix problems, such as dependance tracking, related to global variables you can't just decorate the same globals.

Soma

Code:

reset_event_counter();
increment_event_counter_by_N(42);

I guess I chose some poor wording. It can be misused, quite easily too, if your goal is simply to screw stuff up. My solution isn't bullet-proof, but it does restrict what you can do with event_counter to a small list of things. The point is, the interface doesn't support a "set to any random value". Only increment and reset functions, which is a strong hint to a user that you should only every increase the value or reset it to 0. You should never set it to some random value with no consideration of it's previous state.

As I understood it, the OP was not called/hired to re-design the project, but rather to adopt to the current design and work on it with the rest of the team.

Originally Posted by migf1

As I understood it, the OP was not called/hired to re-design the project, but rather to adopt to the current design and work on it with the rest of the team.

He didn't mention why he was hired, but I suspect you're more or less right. Still, if their current design was a result of ignorance, it can't hurt to explain that it is a poor choice and what some alternatives may be. At least the OP now knows, and can perhaps improve the design should they do some large-scale redesign/refactoring of their product, or they can carry this knowledge into their next job or personal projects.

Originally Posted by anduril462

He didn't mention why he was hired, but I suspect you're more or less right. Still, if their current design was a result of ignorance, it can't hurt to explain that it is a poor choice and what some alternatives may be. At least the OP now knows, and can perhaps improve the design should they do some large-scale redesign/refactoring of their product, or they can carry this knowledge into their next job or personal projects.

I fully agree with that!

anduril462 you nailed it. I now understand that when #define EXT alone is executed, my variables are actually defined. I still do not get why ALL my variabes are suddenly defined once I define EXT to hold a empty string, simply placing EXT before each definition (to me) looks like I now define each variable with an empty string in front of it. This does not conform to C standards? In a .c file, I cannot put an empty string before a variable definition and expect it to suddenly be declared, can I? I am missing something big because I do not see (at all) how declaring EXT to hold an empty string causes the REST of my variables in var.h to be declared, by placing a empty string BEFORE the variable declaration. Thanks again for the input. Also, the codeis for a motor controller, works with many interrupts where variables need to be changed in the interrupts. I should probably run through and see if I can take some of my globals out though. May I ask what the big problem with defining variables like this is? Also, I have been hired to build the motor controller form scratch. I am using an STM32F1 that was previously used to build a particle counter in our company, so I have some of the files from the previous (massive) project to get me started. In other words...changing the code is an option.