To include or not to include.... C language!

[Robby]

Hello,

As you all know I am porting a large program form an 8 bit MCU to a 32 bit
MCU from xxx compiler to Microchip's MPLAB compiler and to include or not to
include files is the issue here today.... and including files is not the same
as I used to do it in the xxx compiler *** unfortunately*** or should I say
FORTUNATELY since this would give me the chance to remold my coding habits of
including files! I am confused. I know you guys helped me alot on code for
that bloody compiler and especially about this include stuff that many times
I had to set aside your reccomendations because the compiler was incapable of
compiling them. :-(

I have begun by porting 6 of the 80 files from xxx compiler to MPLAB and I
have commented out many lines to try to make parts of the program
compile........ hell, disaster, don't even feel like coding anymore, I am
back where I was 3 years ago and can't get the slightest thing to work.... I
now, can't compile anything since the compiler complains of undeclared
variables and warning me that I am illegally declaring variables in a
function's parameter list ....what the "&*@%" is that, first it asked me that
I should pre tag my enum variables with "enum" and now its complaining about
them..... and the list goes on. So I will have to focus on one thing at a
time. I have taken the 6 files from MPLAB and brought them over to VC++ and
compiled them there.

Please note, at some instances there might be lines that look useless, but
they are not, its just because the program has been severely watered down
from its original form and kept only some lines to give the compiler
something to compile.

The error is:

1>C:\_DTS_PROGRAMMING\C_PROGRAMMING\c\C_TESTS\C_FILES\CFILES\Debug\CFILES.exe : fatal error LNK1169: one or more multiply defined symbols found

The thing is that even if I double click on the error, VC++ won't take me to
the line with the problem ????

Here are the 6 files:
====================================KERNEL.h
enum enumKM{KM_QUIT = 0, KM_CREATE = 1, KM_RECUR};

// Enumerate switches 1 to 8 according to their hardware binary input
enum e_keys18 {e_tsFOUR=127, e_tsTHREE=191, e_tsTWO=223, e_tsONE=239,
e_btFOUR=247, e_btTHREE=251, e_btTWO=253, e_btONE=254 };

// Enumerate switches 9 to 12 according to their hardware binary input
enum e_keys912 {e_btNINE=254, e_btTEN=253, e_btELEVEN=251, e_btTWELVE=247};

long MQ[]={1};
long KERNEL_get_msg();
void KERNEL_update_all_inputs();

=========================================KERNEL.c
#include <stdio.h>
#include "KERNEL.h"
#include "API.h"
#include "MAIN.h"

int main(void)
{ACM152_MAIN();}

long KERNEL_get_msg()
{KERNEL_update_all_inputs(); return 1000; }

void KERNEL_update_all_inputs()
{ API_InsertMessage(KM_RECUR); }

======================================MAIN.h
int ACM152_MAIN();

======================================MAIN.c
#include "MAIN.h"
#include "INPUT.h"

int ACM152_MAIN()
{ pUI_DC = &UI_DC; return 0; }

=====================================API.h
void API_InsertMessage(enum enumKM m);

====================================API.c
#include "API.h"
#include "KERNEL.h"

void API_InsertMessage(enum enumKM m)
{ MQ[0] = m; }

===================================INPUT.h
struct USERINPUT_DATA_COLLECTIONS{
int _hed_INPUTKEYS_1_8;
int _hed_INPUTKEYS_9_12;
int _hed_LASTKEY_PRESSED_1_8;
int _hed_LASTKEY_PRESSED_9_12;
int x_Taps_1_8;
int x_Taps_9_12;
} UI_DC, *pUI_DC;

======================================INPUT.c
#include "INPUT.h"

// Intentionally left un coded simplifing sample
===========================================

Now, KERNEL.c has to include KERNEL.h right?

And KERNEL.c has to include API.h since it is calling a function called
"API_InsertMessage()" prototyped in API.h... right?

And KERNEL.c has to include MAIN.h since it is calling a function called
"ACM152_MAIN()" prototyped in API.h... right?

And MAIN.c should include MAIN.h right?

And MAIN.c should include INPUT.h since the two USERINPUT_DATA_COLLECTIONS
structure variables are declared in INPUT.h ... right?

And API.c should include API.h ....right?
And API.c should include KERNEL.h since the MQ array is declared in the
KERNEL.h ... right? P.S. MQ array needs to absolutely be global !!!

And likewise, the INPUT.c file should include its header file ... right.....

QUESTIONS:
1) This including stuff is a mess *to me that is* I still don't see the
advantage of including every .h files in .c files. Why can't we just put
everything that we need to declare in KERNEL.h and make sections denoted by
comments to show the relativity of the declartations? But then we would need
to include KERNEL.h in very .c file right? and this would give duplication
errors at compile time right ??? confused, confused, confused!

2) Keeping the program the way it is, what modification can I do to get rid
of the error?

3) Is the following function okay?

===================================
void API_InsertMessage(enum enumKM m)
{
long MQ_LOC;
MQ[MQ_LOC] = m;
}
==================================

The reason I am asking is that MPLAB generates the following error:

API.c:3: error: parameter `m' has incomplete type

Why would MPLAB complain but VC++ doesn't.... Just though I would mention
that and I know that this question does not pertain to this forum!

But this all used to work without errors in compiler xxx. This makes me
wonder how many things will I have to fix !

I am sincerely thanking anyone who can help me shed some direction in this
"include" stuff as to orient myself to do it right once and for all now that
I have the oppertunity of being in a C compliant compiler!

[Victor Bazarov]

Post by Robby
Hello,
As you all know [..]

Well, I didn't know that, but it probably doesn't matter.

Post by Robby
1>C:\_DTS_PROGRAMMING\C_PROGRAMMING\c\C_TESTS\C_FILES\CFILES\Debug\CFILES.exe : fatal error LNK1169: one or more multiply defined symbols found
The thing is that even if I double click on the error, VC++ won't take me to
the line with the problem ????

Where would it take you? It's a multiple definition of a symbol.

Post by Robby
====================================KERNEL.h
enum enumKM{KM_QUIT = 0, KM_CREATE = 1, KM_RECUR};
// Enumerate switches 1 to 8 according to their hardware binary input
enum e_keys18 {e_tsFOUR=127, e_tsTHREE=191, e_tsTWO=223, e_tsONE=239,
e_btFOUR=247, e_btTHREE=251, e_btTWO=253, e_btONE=254 };
// Enumerate switches 9 to 12 according to their hardware binary input
enum e_keys912 {e_btNINE=254, e_btTEN=253, e_btELEVEN=251, e_btTWELVE=247};
long MQ[]={1};

Here you're defining an array whose name has external linkage. As soon
as you include this header in more than one .c file, you get multiply
defined symbol. Don't do that. Definitions belong to the .c files, not
to headers.

Post by Robby
long KERNEL_get_msg();
void KERNEL_update_all_inputs();
=========================================KERNEL.c
#include <stdio.h>
#include "KERNEL.h"
[..]
======================================MAIN.h
====================================API.c
#include "API.h"
#include "KERNEL.h"
[..]

There you go!...

Post by Robby
===================================INPUT.h
[..]
1) This including stuff is a mess *to me that is* I still don't see the
advantage of including every .h files in .c files.

There is probably no advantage. Include only the headers you actually
need in that module.

Post by Robby
Why can't we just put
everything that we need to declare in KERNEL.h and make sections denoted by
comments to show the relativity of the declartations?

Uh... I don't know. You would have to decide for yourself.

Post by Robby
But then we would need
to include KERNEL.h in very .c file right? and this would give duplication
errors at compile time right ??? confused, confused, confused!

Not at compile time, but at link time (and that's what you get). But
that is easy to fix: just don't put *definitions* in headers, only
*declarations*.

Post by Robby
2) Keeping the program the way it is, what modification can I do to get rid
of the error?

Well, it bears repeating I guess: put only *declarations* in headers.

Post by Robby
3) Is the following function okay?
===================================
void API_InsertMessage(enum enumKM m)
{
long MQ_LOC;
MQ[MQ_LOC] = m;
}
==================================

No. Not OK by a mile. First off, you don't initialize 'MQ_LOC' and
immediately attempt to use it as the index. That's a disaster waiting
to happen. Second, what's 'MQ'? What's 'enumKM'? Did you define
those? Are they known to the compiler at this point?

Post by Robby
API.c:3: error: parameter `m' has incomplete type

Perhaps you forgot to include the header that defines 'enumKM' type...

Post by Robby
Why would MPLAB complain but VC++ doesn't.... Just though I would mention
that and I know that this question does not pertain to this forum!
But this all used to work without errors in compiler xxx. This makes me
wonder how many things will I have to fix !

From the looks of it, enough to keep you busy for a while. But isn't
it actually good? I mean, guaranteed employment an so on...

Post by Robby
[..]

V

[Robby]

Post by Victor Bazarov

Post by Robby
long MQ[]={1};

Here you're defining an array whose name has external linkage. As soon
as you include this header in more than one .c file, you get multiply
defined symbol. Don't do that. Definitions belong to the .c files, not
to headers.

the data in the MQ array has to be accessible and available for the KERNEL.c
and API.c modules. Having said this, (mind you I didn't try this) but, can I
just declare it in KERNEL.h like this:

=====================KERNEL.h
extern long MQ[];

Post by Victor Bazarov

Post by Robby

and have it available in the following modules by doing this?

=====================KERNEL.c
long MQ[10] = {1,2,3,4,5,6,7,8,9,0}; // <<< at top of file!

=====================API.c
#include "API.h"
#include KERNEL.h // <<< include the header with MQ declaration.

void API_InsertMessage(enum enumKM m)
{ MQ[0] = m; }
===========================

Post by Victor Bazarov

Post by Robby
1) This including stuff is a mess *to me that is* I still don't see the
advantage of including every .h files in .c files.

There is probably no advantage. Include only the headers you actually
need in that module.

Okay!

Post by Victor Bazarov

Post by Robby
3) Is the following function okay?
===================================
void API_InsertMessage(enum enumKM m)
{
long MQ_LOC;
MQ[MQ_LOC] = m;
}
==================================

No. Not OK by a mile. First off, you don't initialize 'MQ_LOC' and
immediately attempt to use it as the index. That's a disaster waiting
to happen.

[snip]

Sorry my typo, it was supposed to be:
===================================
void API_InsertMessage(enum enumKM m)
{
long MQ_LOC;
MQ_LOC = KERNEL_find_next_empty_queue_spot(...);
MQ[MQ_LOC] = m;
}
==================================

Post by Victor Bazarov
Second, what's 'MQ'? What's 'enumKM'? Did you define
those? Are they known to the compiler at this point?

Yes, it is defined at the top of KERNEL.h.

Post by Victor Bazarov

Post by Robby
API.c:3: error: parameter `m' has incomplete type

Perhaps you forgot to include the header that defines 'enumKM' type...

But I did include KERNEL.h which is where I declare enumKM type ???

Post by Victor Bazarov
From the looks of it, enough to keep you busy for a while. But isn't
it actually good? I mean, guaranteed employment an so on...

Work is always good. Thanks for the encouragement!
Thank you for your help! If you can, do get back!

Robert