51
votes

DebugUtil.h

#ifndef DEBUG_UTIL_H
#define DEBUG_UTIL_H

#include <windows.h>

int DebugMessage(const char* message)
{
    const int MAX_CHARS = 1023;
    static char s_buffer[MAX_CHARS+1];

    return 0;
}

#endif

When I try to run this I get this error:

Terrain.obj : error LNK2005: "int __cdecl DebugMessage(char const *)" (?DebugMessage@@YAHPBD@Z) already defined in Loodus.obj

Renderer.obj : error LNK2005: "int __cdecl DebugMessage(char const *)" (?DebugMessage@@YAHPBD@Z) already defined in Loodus.obj

test.obj : error LNK2005: "int __cdecl DebugMessage(char const *)" (?DebugMessage@@YAHPBD@Z) already defined in Loodus.obj

C:\Users\Tiago\Desktop\Loodus Engine\Debug\Loodus Engine.exe : fatal error LNK1169: one or more multiply defined symbols found

But why does this happen? I have #ifndef #define and #endif in the header so multiple definitions shouldn't happen

9
@Armen: It's a shame that the answers on this question are much better :(Lightness Races in Orbit

9 Answers

75
votes

Put the definition (body) in a cpp file and leave only the declaration in a h file. Include guards operate only within one translation unit (aka source file), not across all your program.

The One Definition Rule of the C++ standard states that there shall appear exactly one definition of each non-inline function that is used in the program. So, another alternative would be to make your function inline.

14
votes

Make the function inline or declare the function in a header file and define it in a cpp file.

inline int DebugMessage(const char* message)
{
    const int MAX_CHARS = 1023;
    static char s_buffer[MAX_CHARS+1];

    return 0;
}

EDIT:

As a comment by Tomalak Geret'kal suggests, it's better to use my latter suggestions than my former and move the function's declaration to a cpp file.

9
votes

(Assuming the posted code is a header, included from multiple .cpp files)

Header guards do not protect you from link-time multiple definitions. Regardless that you have ensured the header will only appear once per Translation Unit, if you have more than one Translation Unit then that's still multiple definitions.

Write definitions in source files, and only declarations in headers.

The only exceptions are inline functions, functions defined within a class definition (though this is not recommended!) and function templates.

4
votes

This function is included into every translation unit and as a result you get multiple definitions of it - each .obj file contains its own copy. When it's time to link them all together the linker rightfully shows the above error.

You can do a few things:

  1. Move the definition to a .cpp file and keep only the declaration in the header.
  2. Use an anonymous namespace around the function in your header file (but realize it's a hack - you will still have multiple definitions, just no name collision).
  3. Mark it as inline (although it might not always work - only if the compiler actually chooses to inline it). That's also a hack for the same reason as above.
0
votes

Move the definition to a .cpp file.

0
votes

Declare your functions in C++ files. Since you defined your function in the header file, and that header file is included from multiple source files, it gets defined for each source file that includes it. That's why it's reported as being defined in multiple places.

Alternatively, you could make it inline so that the code is inserted wherever it's used instead of being defined as a separate function each time.

0
votes

It looks like you are including DebugUtil.h in more than one translation unit, then linking those objects together. However, DebugUtil.h provides a definition for the DebugMessage function, so that definition exists in all of the translation units that incorporated the header. As a result, when you link the objects, the linker rightly complains that the symbol is multiply defined.

Change DebugUtil.h so that it declares DebugMessage via a prototype, but does not provide a definition, and place the definition of DebugMessage in a .c file which you will compile and link with your other objects.

0
votes

That only prevents multiple inclusions in the same source file; multiple source files #includeing it will still generate multiple definitions of DebugMessage(). In general, you should either not place functions in header files at all or make them static (and usually inline, since otherwise it doesn't usually make sense to have multiple static definitions of the same function).

0
votes

100% Certain you correctly included Guards but still getting redefinition error?

For Visual Studio: I was really frustrated because I was correctly included guards, only to find out the problem was visual studio. If you have added the file to your project, the compiler will add the file twice even if you have include guards around your implementation file and header file.

If you don't use visual studio exclusively, and say... use code::blocks sometimes, you might want to only #include the file when you detect the absence of the visual studio environment.

DebugUtil.h :
----------------------
#ifndef _WIN32
#include "DebugUtil.c"
#endif
----------------------

If you are okay with including stdio.h, you can be a little less hackish about it:

DebugUtil.h :
----------------------
#include <stdio.h>
#ifdef _MSC_VER
#include "DebugUtil.c"
#endif
----------------------

Reference: Predefined Macros, Visual Studio: https://msdn.microsoft.com/en-us/library/b0084kay.aspx