Thread: Strange error and impending sanity loss

  1. #1
    Registered User
    Join Date
    Nov 2008
    Posts
    11

    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.

  2. #2
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    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.

  3. #3
    Registered User
    Join Date
    Nov 2008
    Posts
    11
    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.

  4. #4
    Registered User
    Join Date
    Nov 2008
    Posts
    11
    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.

  5. #5
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  6. #6
    Registered User
    Join Date
    Nov 2008
    Posts
    11
    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!

  7. #7
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    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.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  8. #8
    Registered User
    Join Date
    Nov 2008
    Posts
    11
    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?

  9. #9
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    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.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  10. #10
    Malum in se abachler's Avatar
    Join Date
    Apr 2007
    Posts
    3,195
    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
    Last edited by abachler; 11-28-2008 at 08:56 AM.

  11. #11
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by abachler View Post
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  12. #12
    Registered User
    Join Date
    Nov 2008
    Posts
    11
    Thanks for the amazingly pedagogical explanations, though to be perfectly honest I didn't really understand the purpose of what you described abachler.

    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?

  13. #13
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    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 Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  14. #14
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by Maer View Post
    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?

  15. #15
    Registered User
    Join Date
    Nov 2008
    Posts
    11
    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.

Popular pages Recent additions subscribe to a feed