LocustSpectre
LocustSpectre

Reputation: 87

Link error during gcc compile with makefile

(Before moving on,
the absence of frees and writing variables which are never used in the codes,
are intended for testing a tool)

I wrote codes like these and Makefile like this:

unread_two.h

#ifndef __UNREAD_TWO_H
#define __UNREAD_TWO_H

const int SIZEOF_INT = sizeof(int);
int addTwo();

unread_twomain.c

#include <stdio.h> 
#include <stdlib.h>
#include "unread_two.h"

int main(int argc, char *argv[])
{
    int *x;

    x = (int *)malloc(SIZEOF_INT);
    x = addTwo();
    free(x);

    return 0;
}

unread_two.c

#include <stdio.h>
#include <stdlib.h>
#include <unread_two.h>

int addTwo()
{
    int *y, *z, sum;
    y = (int *)malloc(SIZEOF_INT);
    z = (int *)malloc(SIZEOF_INT);

    *y = 3;
    sum = *y + *y;
    return sum;
}

Makefile

CC=gcc
CCFLAGS=-g

%.o: %.c
    $(CC) -c $< $(CCFLAGS)

all: unread_two
clobber: clean
    rm -f *~ \#`\# core

clean:
    rm -f unread_two *.o

unread_two: unread_twomain.o unread_two.o

unread_twomain.o: unread_two.h

unread_two.o: unread_two.h

and when I put make all, this message appears:

unread_twomain.o:(.rodata+0x0): multiple definition of `SIZEOF_INT'
unread_two.o:(.rodata+0x0): first defined here
collect2: error: ld returned 1 exit status

What things should I fix?

Upvotes: 2

Views: 392

Answers (2)

Some programmer dude
Some programmer dude

Reputation: 409136

You actually have two errors, the one you report and another more devious bug.

The problem you are getting an error about is that the constant SIZEOF_INT is defined in all source file you include the header file in. Include guards only protect against multiple inclusion in the same source file (or translation unit technically speaking), but not agains you including the same file in multiple sources. This means that the compiler will create a definition of SIZEOF_INT in both unread_twomain.o and unread_two.o, which the linker will then complain about.

The solution to this is to only declare the constant in the header file, and then define it in a single source file.


The other problem is that in main you create x as a pointer, and allocate memory for it (by the way, you should not typecast the return of malloc), and then assing the result from addTwo to this pointer. But addTwo doesn't return a pointer, it returns a straight value, so you make the pointer x point to the address 6 which is not exactly what you intend to do I guess. This will cause undefined behavior when you then try to free the memory pointed to by x, most likely causing a crash.

In your program you don't have to use pointers at all. Just use normal non-pointer variables:

int addTwo()
{
    int y = 3;
    int sum = y + y;

    return sum;
}

And

int main(int argc, char *argv[])
{
    int x = addTwo();

    return 0;
}

Upvotes: 2

Paul R
Paul R

Reputation: 212929

You shouldn't be defining SIZEOF_INT in a header, otherwise you'll get multiple definitions when you include this header in more than one compilation unit, as you have seen. Instead declare it in a header and define it in a source file:

// unread_two.h

extern const int SIZEOF_INT;         // *declare* SIZEOF_INT


// unread_two.c

const int SIZEOF_INT = sizeof(int);  // *define* SIZEOF_INT


Alternatively in this particular case you might be justified in doing it the "old skool" way, with a macro:

// unread_two.h

#define SIZEOF_INT sizeof(int)

Upvotes: 4

Related Questions