Strange error and impending sanity loss

This is my first ever post in a programming forum, so please, be gentle. ;) Obviously I'm new to this.

I got a sudden and confusing error when trying to compile. As always, I assume it's something simple I just can't see, so I'm hoping someone could point out the "duh" for me. It could be the late hour or the horrible timing, but this is just driving me nuts.

make -k all
gcc -Wall -o testapp -Iinclude SDL.dll zlib1.dll SDL_ttf.dll libfreetype-6.dll main.c triangle.c drawline.c object.c list.c text.c highscore.c
/cygdrive/c/Users/M/AppData/Local/Temp/ccDwoHhY.o:highscore.c: (.data+0x0): multiple definition of `_highscore'
/cygdrive/c/Users/M/AppData/Local/Temp/ccjdOM2f.o:main.c: (.data+0x20340): first defined here
/cygdrive/c/Users/M/AppData/Local/Temp/ccDwoHhY.o:highscore.c: (.data+0x2c): multiple definition of `_currscore'
/cygdrive/c/Users/M/AppData/Local/Temp/ccjdOM2f.o:main.c: (.data+0x2036c): first defined here
collect2: ld returned 1 exit status
make: *** [testapp] Error 1
make: Target `all' not remade because of errors.


Hopefully relevant code:

Highscore.h

Code:

---


#ifndef HIGHSCORE_H_
#define HIGHSCORE_H_


int highscore[11] = {1,1,2,3,4,5,6,7,8,9,9};
int currscore = 12;
char buf[20];


void InsertScore(int highscore[]);

int SaveHighscore(int highscore[]);
 
void LoadHighscore(int highscore[]);

void PrintHighscore();


#endif /*HIGHSCORE_H_*/

---

highscore.c

Code:

---


#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include "highscore.h"


void InsertScore(int highscore[])
{
 
        if(currscore >= highscore[10]){
        highscore[10] = currscore;
    }
   
   
}

               

// open file
int SaveHighscore(int highscore[])
{
    FILE *fil;

    fil = fopen("highscore.txt", "w");
    if (fil == NULL) {
        printf("Unable to open file\n");
        goto error;
                }
        if (fwrite(highscore, sizeof(highscore), 10, fil) != 1){
                printf("Unable to write to file\n");
                goto error;
                }
        fclose(fil);
       
        return 1;
       
        error:
                return 0;
}


void LoadHighscore(int highscore[])
{
        FILE *stream;
       
   
       
        stream = fopen("highscore.txt", "r");
       
       
    fread(highscore, sizeof(highscore), 10, stream);
        fclose(stream);
       
}

---

I'm not adding anything from main.c because neither the highscore nor the currscore is actually anywhere in it, which makes the error even stranger to my poor newbie mind.
I won't go so far as to say this worked like a charm before the strange error (kind of the opposite, in fact), but it was at least a bit less confusing.

Any help is muchly appreciated.

But if main.c includes highscore.h, then highscore is most definitely "in it". Every .c file that includes highscore.h gets its very own copy of the highscore array -- which means that they cannot later be linked together, since they each have their very own copy of the highscore array.

If you really need this global array, you would have to put a static declaration in the header file (as in "static int highscore[11]", and then in one file (presumably main.c) declare a global variable with initializer.

All right. that makes sense, I didn't realize the implications. I moved the array to highscore.c, and it took care of the error. I'm sure other stuff will crop up, but hey. :)

Thank you for the help.

I take it back, I'm still confused. If I declare the array in highscore.c and then try to call the function LoadHighscore in main, it says the array isn't declared. And that's fair enough, but as calling it in main won't work either, I really don't know how to go about this. Other than putting all the functions I made in highscore.c in main, but that's cluttered and generally not very satisfying.

Right, so you declare the array "extern int highscore[];" in the highscore.h file, and then define the variable in one of the .c files that include highscore.h.

Remember, include files are not magical. It's just a pretty simple way to replace a short text (#include "something.h") with the contents of the file. It would have exactly the same effect if you typed the whole contents of highscore.h into the two different source files - it would just make the work of changing any of it much harder.

--
Mats

I finally got it to work. It even does what it's supposed to, which is just mind boggling right now. :P

Thank you so much!

It's quite simple, really.
When you put the definition of the variable in the the header, you got two arrays, in two files. The linker sees two arrays which are global by the same name and complains.
The second option is putting the array in one file to avoid the problem. Yet the compiler does not know that that array exists somewhere, so you have to help it. You do this by declaring it exists, by putting "extern" in front of it. Typically, it's placed in headers, so that any source file that includes that header can use that global variable.
Since there's only one global variable, the linker is happy, and as the compiler can see that the variable exists, but possibly in another source file, it, too, is happy. So it works.

Thank you Elysia for the very nice explanation. :)

Seems I'm on a roll here, so perhaps you wouldn't mind helping me some more.

I need to write the array to a file, but I can't get the function to work. I know it opens right because the LoadHighscore function works and I get no error messages when trying to open the file in SaveHighscore. I've also checked that the file is actually write enabled.

I've also added another array along the lines of highscore[] called test so that I'll know it works properly if it's printed with the LoadHighscore (which should mean that the highscore array has been replaced with the test one). Anyway, that's all academic as I can't get it to write in the first place.

highscore.c with the SaveHighscore function

Code:

---


// open file
int SaveHighscore(int *test)
{
    FILE *fil;


    fil = fopen("highscore.txt", "w");
    if (fil == NULL) {
        printf("Unable to open file\n");
        goto error;
                }
        if (fwrite(&test, sizeof(test[0]), 11, fil) != 1){
                printf("Unable to write to file\n");
                goto error;
                }
        fclose(fil);
       
        return 1;
       
        error:
                return 0;
}


void LoadHighscore(int *highscore)
{
        FILE *stream;
        int i;
   
       
        stream = fopen("highscore.txt", "r");
       
   
        if(stream==NULL) {
            printf("Error: can't open file.\n");
          }
  else {
    printf("File opened.\n");
  }
       
    fread(highscore, sizeof(highscore), 10, stream);
    for(i=0; i<10; i++){
    printf("File read: %d \n", highscore[i]);
    }
        fclose(stream);
       
}

---

main.c

Code:

---


#include <stdlib.h>
#include <stdio.h>
#include <math.h>
#include <unistd.h>
#include <assert.h>
#include "SDL.h"
#include "SDL_ttf.h"
#include "drawline.h"
#include "triangle.h"
#include "teapot_data.h"
#include "sphere_data.h"
#include "object.h"
#include "text.h"
#include "list.h"
#include "highscore.h"


int highscore[11] = {1,1,2,3,4,5,6,7,8,9,9};
int test[11] = {2,2,2,2,2,2,2,2,2,2,2};

(snip)


  // Start game
    while (1) {
       
        SaveHighscore(test);
        LoadHighscore(highscore);

---

If I move the entire int SaveHighscore into main and change the 3rd argument in fwrite from 11 to 1 it'll print out
File opened.
File read. 4342656
File read: 1
File read: 2
etc.

What's that all about?

Any suggestions?

Not surprisingly, you don't grasp the concepts of pointers fully.

Code:

---

if (fwrite(&test, sizeof(test[0]), 11, fil) != 1){

---

So what's wrong with this? Well, firstly you are passing the address of the pointer itself - meaning that fwrite should write the address stored in the pointer, not the actual data. Oh no.
Secondly, you tell fwrite to write the size of the type it points to, since all elements in an array are the same size, test[0] is the same size as test[1], etc.
And that size is not necessarily the size of a pointer. Oh no. Undefined behavior right there.
Thirdly, you tell it to write 11 items, but again, you are trying to write the value of the pointer itself, which does not have 11 elements, nor necessarily the size of test[0]. Oh no.

Code:

---

fread(highscore, sizeof(highscore), 10, stream);

---

So what's wrong with this?
OK, so it's slightly more correct, but sizeof(highscore) will return the size of the pointer, not the size of the actual data! Oh no.
Secondly, you want to read 11 elements, no?
Thirdly, you don't know the actual size of the array passed into the function (using sizeof of a dereferences pointer will give you the size of one element, not how many elements the pointer may be pointing to), so you may write more data than you have space. Oh no.

Well, that about sums it up.
You might also want to fix your indentation.

another method is to use #defines to restrict the definition to once, there are also pragamas that are compiler specific

Code:

---

// this goes in highscore.h

#ifdef _HIGHSCORE_H
extern int highscore[];
#else
#define _HIGHSCORE_H
int highscore[] = {1,1,2,3,4,5,6,7,8,9,9};
#endif

---

Quote:

---

Originally Posted by abachler

another method is to use #defines to restrict the definition to once, there are also pragamas that are compiler specific

Code:

---

// this goes in highscore.h

#ifdef _HIGHSCORE_H
extern int highscore[];
#else
#define _HIGHSCORE_H
int highscore[] = {1,1,2,3,4,5,6,7,8,9,9};
#endif

---

---

But that doesn't solve the problem, as it would be included in both main.cpp and highscore.cpp as "int highscore[] ... ". Only if you include the header twice will it make a difference - but that is solved anyways if it's extern, and it's wrong to define it in the second way.

--
Mats

Thanks for the amazingly pedagogical explanations, though to be perfectly honest I didn't really understand the purpose of what you described abachler.

Quote:

---

Not surprisingly, you don't grasp the concepts of pointers fully.
Code:

if (fwrite(&test, sizeof(test[0]), 11, fil) != 1){

So what's wrong with this? Well, firstly you are passing the address of the pointer itself - meaning that fwrite should write the address stored in the pointer, not the actual data. Oh no.

---

I don't understand pointers fully, that I'll agree to, but I know enough that I was thoroughly confused when several old forum posts and online articles told me to write it like that. I just blamed it on my giant newbie-ness and went with the so-called expert advice. :)

But knowing what I'm doing wrong doesn't make it right. How am I supposed to write this? And do you have any idea why LoadHighscore works when it's in highscore.c, but I had to move SaveHighscore to main.c?

Well, usually you would write it like that when you want to write data that is not a pointer.
Like

Code:

---

int x;
fwrite(&x, sizeof(x), 1, file);

---

Since fwrite wants a pointer to the data it should write.
But if you have a pointer that points to the data to write then... wait, you have the pointer, don't you? So you don't want to write the pointer, but the pointee!

Quote:

---

Originally Posted by Maer

Thanks for the amazingly pedagogical explanations, though to be perfectly honest I didn't really understand the purpose of what you described abachler.



I don't understand pointers fully, that I'll agree to, but I know enough that I was thoroughly confused when several old forum posts and online articles told me to write it like that. I just blamed it on my giant newbie-ness and went with the so-called expert advice. :)

But knowing what I'm doing wrong doesn't make it right. How am I supposed to write this? And do you have any idea why LoadHighscore works when it's in highscore.c, but I had to move SaveHighscore to main.c?

---

The examples given assumed you had an object. You don't have an object, you have a pointer to an object. Since the first parameter of fwrite is supposed to be a pointer to an object, then you are done.

You don't have to move SaveHighscore to main.c. Why do you think you did?

Quote:

---

You don't have to move SaveHighscore to main.c. Why do you think you did?

---

Just because it wouldn't work at all when I didn't. But that was with the pointer confusion. I'll move it back and try again. :)

Well, either you lost me somewhere along the very short road, or something else is not as it should be, because I still can't get it to write properly.

Why not show your current attempt?
I'm sure I can punch a few holes through it.

Hehe, I'm sure you can punch many, and you're welcome to as long as I can patch them up again. :D

I didn't change much from the code I posted aside from the two you pointed out. Basically

Code:

---

    if (fwrite(test, sizeof(test[0]), 11, fil) != 1){

---

and

Code:

---

    fread(highscore, sizeof(highscore[0]), 11, stream);

---

Quote:

---

Secondly, you tell fwrite to write the size of the type it points to, since all elements in an array are the same size, test[0] is the same size as test[1], etc.
And that size is not necessarily the size of a pointer. Oh no. Undefined behavior right there.

---

I'm not clear on what you meant with this, so I didn't change it. I thought declaring the size of the element is what I wanted to do.

Quote:

---

Originally Posted by Maer

Hehe, I'm sure you can punch many, and you're welcome to as long as I can patch them up again. :D

I didn't change much from the code I posted aside from the two you pointed out.

I'm not clear on what you meant with this, so I didn't change it. I thought declaring the size of the element is what I wanted to do.

---

But that doesn't help us know what you've changed, from which version of what was posted, etc.

As to the second, you want to declare the size of the element. Using sizeof(highscore) doesn't give you the size of the element, since highscore isn't an element, it's a pointer.

Quote:

---

But that doesn't help us know what you've changed, from which version of what was posted, etc

---

Finally, something I can fix. ;)



highscore.h

Code:

---

#ifndef HIGHSCORE_H_
#define HIGHSCORE_H_

extern int highscore[];
extern int test[];


//void InsertScore(int *highscore);

int SaveHighscore(int *test);
 
void LoadHighscore(int *highscore);


#endif /*HIGHSCORE_H_*/

---

highscore.c

Code:

---

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include "highscore.h"

// open file
int SaveHighscore(int *test)
{
    FILE *fil;


    fil = fopen("highscore.txt", "w");
    if (fil == NULL) {
        printf("Unable to open file\n");
        goto error;
        }
    fwrite(test, sizeof(test[0]), 11, fil);   
    if (fwrite(test, sizeof(test[0]), 11, fil) != 1){
        printf("Unable to write to file\n");
        goto error;
        }
    fclose(fil);
   
    return 1;
   
    error:
        return 0;
}


void LoadHighscore(int *highscore)
{
    FILE *stream;
    int i;
   
   
    stream = fopen("highscore.txt", "r");
   
   
    if(stream==NULL) {
        printf("Error: can't open file.\n");
    }
  else {
    printf("File opened.\n");
  }
       
    fread(highscore, sizeof(highscore[0]), 11, stream);
    for(i=0; i<11; i++){
    printf("File read: %d \n", highscore[i]);
    }
    fclose(stream);
   
}

---

main.c

Code:

---

#include <stdlib.h>
#include <stdio.h>
#include <math.h>
#include <unistd.h>
#include <assert.h>
#include "SDL.h"
#include "SDL_ttf.h"
#include "drawline.h"
#include "triangle.h"
#include "teapot_data.h"
#include "sphere_data.h"
#include "object.h"
#include "text.h"
#include "list.h"
#include "highscore.h"

(snip)



int highscore[11] = {1,1,2,3,4,5,6,7,8,9,9};
int test[11] = {2,2,2,2,2,2,2,2,2,2,2};

(snip)

int main(int argc, char **argv)
{
    // Start game
    while (1) {
 

       
        SaveHighscore(test);
        LoadHighscore(highscore);

---

A few security addments, a few fixes and indentation fixing.

Highscore.h:

Code:

---

#ifndef HIGHSCORE_H_
#define HIGHSCORE_H_

//Avoid global variables
//extern int highscore[];
//extern int test[];

int SaveHighscore(int* test, int size);
void LoadHighscore(int* highscore, int size);

#endif /*HIGHSCORE_H_*/

---

Highscore.c:

Code:

---

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include "highscore.h"

// open file
int SaveHighscore(int* test, int size)
{
        FILE* fil = fopen("highscore.txt", "w");
        if (fil == NULL)
        {
                printf("Unable to open file\n");
                return 0;
        }
        //fwrite(test, sizeof(test[0]), 11, fil);
        if (fwrite(test, sizeof(test[0]), size, fil) != 1)
        {
                printf("Unable to write to file\n");
                return 0;
        }
        fclose(fil);
        return 1;
}

void LoadHighscore(int* highscore, int size)
{
        FILE* stream = fopen("highscore.txt", "r");
        if(stream == NULL)
        {
                printf("Error: can't open file.\n");
                return;
        }
        else
        {
                printf("File opened.\n");
        }

        fread(highscore, sizeof(highscore[0]), size, stream);
        for (int i = 0; i < 11; i++)
        {
                printf("File read: %d \n", highscore[i]);
        }
        fclose(stream);
}

---

Main.c:

Code:

---

#include <stdlib.h>
#include <stdio.h>
#include <math.h>
#include <unistd.h>
#include <assert.h>
#include "SDL.h"
#include "SDL_ttf.h"
#include "drawline.h"
#include "triangle.h"
#include "teapot_data.h"
#include "sphere_data.h"
#include "object.h"
#include "text.h"
#include "list.h"
#include "highscore.h"

int highscore[11] = {1, 1, 2, 3, 4, 5, 6, 7, 8, 9, 9};
int test[11] = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2};

int main(int argc, char** argv)
{
        // Start game
        for(;;)
        {
                SaveHighscore(test, sizeof(test) / sizeof(test[0]));
                LoadHighscore(highscore, sizeof(highscore) / sizeof(highscore[0]));
        }
}

---

Thanks for all the help, it's muchly appreciated. I finally got it to work. :)