3

I work with several files: main.c assembler.c fileHandlers.c handlers.c in addition I have several header files, containing constants, function prototypes etc. In one of them ("datatypes.h") I defined an array of strings:

#ifndef DATATYPES_H
#define DATATYPES_H
const char *OPCODES = {"str1",..., str15}.
#endif

Then, I included this header in all my files (since they all use it at some point).

This is my makefile:

main: main.o assembler.o filesHandler.o handlers.o
    gcc -g -Wall -ansi -pedantic main.o assembler.o filesHandler.o handlers.o -o main

main.o: main.c
    gcc -g -c -Wall -ansi -pedantic main.c -o main.o

assembler.o: assembler.c
    gcc -g -c -Wall -ansi -pedantic assembler.c -o assembler.o

filesHandler.o: filesHandler.c
    gcc -g -c -Wall -ansi -pedantic filesHandler.c -o filesHandler.o

handlers.o: handlers.c
    gcc -g -c -Wall -ansi -pedantic handlers.c -o handlers.o

When I try to compile, I get the following error:

gcc -g -c -Wall -ansi -pedantic assembler.c -o assembler.o
gcc -g -c -Wall -ansi -pedantic filesHandler.c -o filesHandler.o
filesHandler.c: In function ‘readFile’:
filesHandler.c:14:10: warning: unused variable ‘addressing’ [-Wunused-variable]
     char addressing[MAXWORD];
          ^
gcc -g -Wall -ansi -pedantic main.o assembler.o filesHandler.o handlers.o -o main
assembler.o:(.data+0x0): multiple definition of `OPCODES'
main.o:(.data+0x0): first defined here
filesHandler.o:(.data+0x0): multiple definition of `OPCODES'
main.o:(.data+0x0): first defined here
handlers.o:(.data+0x0): multiple definition of `OPCODES'
main.o:(.data+0x0): first defined here
collect2: error: ld returned 1 exit status
make: *** [main] Error 1

Now, I understand that for some reason, the array is being defined more than once after preprocessing, but I don't know why. I read the Wikipedia article on #include guards, and some more resources out there. They all instruct to do just as I did, but it just doesn't work.

Sorry for loading so much data, but I hope it will prevent unnecessary followups.

Thanks, Elad

Elad Edri
  • 331
  • 2
  • 9
  • The second line `#include DATATYPES_H` should be `#define DATATYPES_H`. If the flag is not defined, we haven't been included yet, so define it. If it is defined, we have already been included once, so do nothing. – BoBTFish Mar 17 '16 at 14:47
  • Sorry, I mistype the question. In the file it is #define, not #include, but it still doesn't work. I fixed the question. – Elad Edri Mar 17 '16 at 14:50
  • You could just make OPCODES itself const. `const char * const OPCODES = ...`. – Niall Mar 17 '16 at 19:29

3 Answers3

5

With proper include guards (which yours seem to be, now you have updated the question), a header file is only included once in each translation unit.

A translation unit is basically each .cpp file, with all of the header files it #includes copied into it. Each .cpp file is compiled separately into an object file (.o).

The object files are then linked into the final binary.

So the code in each header is seen in every object file, then multiple times in the final binary. This is fine if you only have declarations in the header file, e.g.

const char* opcodes;

Every translation unit knows there is a string called opcodes, but doesn't try to create the actual variable.

But if you have definitions in the header file, this will be seen multiple times, which is not allowed, and every translation unit will try to create the actual variable:

const char* opcodes = "foobar";

You should instead put the declaration in the header file and use the extern keyword, so that every translation unit can see the name, and make use of it, but put the definition in a single .cpp file, so it is only actually created once, and everyone else refers to the same instance.

// foo.h included everywhere
#ifndef FOO_H
#define FOO_H
extern const char* opcodes;
#endif

.

// foo.cpp
#include "foo.h"
const char* opcodes = "foobar";

Working example:

// a.h
#ifndef A_H
#define A_H
extern const char* foo;
#endif

.

// a.cpp
#include "a.h"
const char* foo = "Foo";

.

// main.cpp
#include <iostream>
#include "a.h"
int main() {
    std::cout << foo << '\n';
}

Then I compile and link in separate steps:

$ g++492 --std=c++14 -Wall -W -Werror -c a.cpp -o a.o
$ g++492 --std=c++14 -Wall -W -Werror -c main.cpp -o main.o
$ g++492 --std=c++14 main.o a.o -o main.tsk
$ ./main.tsk 
Foo

Removing the extern I can compile main.cpp, but not a.cpp: error: redefinition of 'const char* foo'.

Then if I try to link the new main.o with the old a.cpp, I get and error there too, more like your original: multiple definition of 'foo'.

BoBTFish
  • 19,167
  • 3
  • 49
  • 76
  • I did as you suggested - one note though - It didn't work with 'extern', when I removed it, everything works fine - Thank you ALL! – Elad Edri Mar 17 '16 at 15:17
  • Strange, without the `extern` I'd expect a complain like `error: redefinition of 'const char* opcodes'`. I'll edit in an example that I am testing right now, and I know works for sure. – BoBTFish Mar 17 '16 at 15:29
  • This makes sense, but why then is a pound define allowed to have multiple definitions? E.g. `#ifndef FOO_H #define FOO_H #define FOO "Foo" #endif` – corym Oct 01 '19 at 21:01
  • @cdamayor I don't really understand your question; your example shows `FOO_H` and `FOO` defined once each. If you mean these symbols will end up appearing multiple times in the linked binary, that isn't true, the preprocessor simply does source replacment, the names `FOO_H` and `FOO` no longer exist by the time of compilation or linking. Maybe you need to create a new question with more detail and a more complete example. – BoBTFish Oct 02 '19 at 05:54
  • Actually that answers my question thanks. I forgot how the preprocessor works... – corym Oct 03 '19 at 16:09
0

That's a variable definition, and its default linkage is external.

That means that you have one definition with external linkage in each translation unit where you include the header.

Change the header contents to only contain the declaration

extern const char *OPCODES;

and put the definition in exactly one source file, not a header.

molbdnilo
  • 64,751
  • 3
  • 43
  • 82
0

You should declare OPCODES in header, and define it in source file.

datatypes.h

const char *OPCODES;

datatypes.c or datatypes.cpp

const char *OPCODES = {"str1",..., str15};

Or the symbol of 'OPCODES' will be generated in all .o files include the 'datatypes.h', and then linking error.

owent
  • 118
  • 4