Thread: Segmentation faults out the ****

  1. #1
    Registered User
    Join Date
    Sep 2005
    Posts
    15

    Angry Segmentation faults out the ****

    i have the following:


    Code:
    myfile=fopen("getlin.txt","r+");
    char*compare=(char*)malloc(sizeof(char*)*20);
    int*choice=(int*)malloc(sizeof(int*)*2);
    scanf("%d",choice);
    printf("\nWhat would you like to search for?");
    scanf("%s",compare);
    printf("%s\n",compare);
    ReadCatalogue(myfile,*choice,*compare);    /*Call to function, ReadCatalogue, (this is were it segments)*/
    
    
    
    void ReadCatalogue(FILE *myfile, int choose, char**compare)     /*The called function, ReadCatalogue*/
    is anyone able to suggest a reason for this?

    i know it has something to do with my arguments when calling ReadCatalogue cause i get this warning when compiling

    warning: passing arg 3 of `ReadCatalogue' makes pointer from integer without a cast.

    this wasnt covered in the lecture notes and i havent a clue what it means.

    Also before you go and make fun at my stupid errors, remember i have been using C for less than 20 hours total.

    Thanx for your help and please post before you die of laughter cause i need a solution.

  2. #2
    Nonconformist Narf's Avatar
    Join Date
    Aug 2005
    Posts
    174
    Your compare variable is a pointer, but ReadCatalogue expects a pointer to a pointer. Change the call to this and see how it works:
    Code:
    ReadCatalogue(myfile,*choice,&compare);
    Just because I don't care doesn't mean I don't understand.

  3. #3
    Registered User
    Join Date
    Sep 2005
    Posts
    15
    nope soz dude that didnt help at all, still segmenting.
    Last edited by Ubber_C_Noob; 09-10-2005 at 06:57 PM.

  4. #4
    Nonconformist Narf's Avatar
    Join Date
    Aug 2005
    Posts
    174
    i now give you permission to laugh at my code
    Well I would, if there were something to laugh at. By the way, I have a way to call malloc() that's easier to get right:
    Code:
    char*compare=(char*)malloc(sizeof(char*)*20);
    int*choice=(int*)malloc(sizeof(int*)*2);
    Becomes:
    Code:
    char *compare = malloc(20 * sizeof *compare);
    int *choice = malloc(2 * sizeof *choice);
    Your calls should have used sizeof(char) and sizeof(int), respectively. But with my way you don't have to worry about that.
    Just because I don't care doesn't mean I don't understand.

  5. #5
    Registered User
    Join Date
    Sep 2005
    Posts
    15
    ok, i chagnged it so that you dont get the compare string in the main function, but in the ReadCatalouge function, thus reducing the amount of things being passed to ReadCatalogue.

    segementation area is highlighted in blue.

    Code:
    myfile=fopen("getlin.txt","r+");
    int*choice=(int*)malloc(sizeof(int*)*2);
    scanf("%d",choice);
    ReadCatalogue(myfile,1); 
    
    void ReadCatalogue(FILE *myfile, int choose)
    char*compare=(char*)malloc(sizeof(char*)*20);
    printf("\nWhat would you like to search for?");
    scanf("%s",compare);
    i have highlighted in red the part i think is causeing the error.

  6. #6
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    So show us the ReadCatalogue function, and not just your call to it.


    Quzah.
    Hope is the first step on the road to disappointment.

  7. #7
    Registered User
    Join Date
    Sep 2005
    Posts
    15
    the above post is no longer valid, ive change the code soo much.

  8. #8
    Registered User
    Join Date
    Sep 2005
    Posts
    15
    ok heres the new porblem.

    Code:
    char*Catalogue = (char*)malloc(sizeof(char*)*10);
    scanf("%s",&Catalogue);
    if((myfile = fopen(&Catalogue,"r+"))==NULL)
    {
       printf("\nFile could not be loaded.\n");
    }
    else
    {
       printf("\nFile opened and has been read.\n");
       ReadCatalogue(myfile);
       fclose(myfile);
    }
    printf("hehe");
    this is inside a loop whos condition is independant of the file useage, it is controled by what number a user selects earlier in the program and thus should not be affected by the opening of the file.

    i input text.txt, which exists and has the following text in it.

    Code:
    reddas
    red,da
    25/04/1986
    4.55
    the output when this is run is:

    Code:
    File opened and has been read.
    hehe11>
    why does it close the program?

    it should return to the top of the loop and ask me to select another option.

    please help, i know that it calls ReadCatalogue, reads the stuff from the .txt file and returns to main, but it quits on me and that isnt acceptable.
    Last edited by Ubber_C_Noob; 09-11-2005 at 12:03 AM.

  9. #9
    Yes, my avatar is stolen anonytmouse's Avatar
    Join Date
    Dec 2002
    Posts
    2,544
    Code:
    char*Catalogue = (char*)malloc(sizeof(char*)*10);
    scanf("%s",&Catalogue);
    %s with scanf expects a char*. Therefore, you should not give it the address of Catalogue which makes a char**. You had this correct in your original code.

    Also, when using %s you should specify a maximum number of characters to read. Otherwise, scanf can read an unlimited number of characters, possibly overflowing your buffer.

    malloc is generally not required when you want to create a fixed size allocation. Just use an array.

    Finally, it's a good idea to use spaces in your code. This makes it much easier to read.
    Code:
    char Catalogue[50];
    scanf("%49s", Catalogue);
    ~~~~
    Code:
    if((myfile = fopen(&Catalogue,"r+"))==NULL)
    Again, fopen expects a char* so don't give it the address of Catalogue.
    Code:
    if((myfile = fopen(Catalogue, "r+")) == NULL)

  10. #10
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Quote Originally Posted by Ubber_C_Noob
    ok heres the new porblem.
    Code:
    char*Catalogue = (char*)malloc(sizeof(char*)*10);
    scanf("%s",&Catalogue);
    Well it's two of them anyway. No, make that more...

    1) Catalogue is a pointer to a character. You allocate 10 pointers to characters, not 10 characters.
    2) You typecast malloc. Which if this was right, would be wrong. So really you end up allocating space for 10 pointers to characters, but you treat them as characters. I suppose technically you and get away with that, but it's still wrong.
    3) You don't use & when you already have a pointer that you're using with scanf. You're passing it the address of a pointer, not a pointer.
    Quote Originally Posted by Ubber_C_Noob
    Code:
    if((myfile = fopen(&Catalogue,"r+"))==NULL)
    {
       printf("\nFile could not be loaded.\n");
    }
    You're passing the address of a pointer again, not the pointer itself...
    Quote Originally Posted by Ubber_C_Noob
    Code:
    else
    {
       printf("\nFile opened and has been read.\n");
       ReadCatalogue(myfile);
       fclose(myfile);
    }
    printf("hehe");
    No, it actually hasn't been read at this point. Also, wait a second...

    Quote Originally Posted by quzah
    So show us the ReadCatalogue function, and not just your call to it.
    Yeah, I still don't see ReadCatalogue any place...


    Quzah.
    [edit] Beaten twice. This is what happens when you harvest felled high plains arbor while you type... [/edit]
    Hope is the first step on the road to disappointment.

  11. #11
    Registered User
    Join Date
    Sep 2005
    Posts
    15
    ok, you wanted the function ReadCatatlogue here it is:

    Code:
    void ReadCatalogue(FILE *myfile)
    {
       char*loadedfile=(char*)malloc(sizeof(char*)*600);
       char*temp = (char*)malloc(sizeof(char*)*20);
       while(fgets(temp,sizeof(temp),myfile))
       {
          strcat(loadedfile,temp);
       }
       printf(loadedfile);
    }
    this i know is correct and i know that the file has been read cause i get a print out of the file before the thing quits on me.

    that printf() statment above is supposed to tell the user that the file has been opened and read only if it was able to be opened, since were only useing txt files i dont see how you can open the file and not be able to read it in the future.
    Last edited by Ubber_C_Noob; 09-11-2005 at 12:48 AM.

  12. #12
    Registered User
    Join Date
    Sep 2004
    Location
    California
    Posts
    3,268
    Code:
    char*loadedfile=(char*)malloc(sizeof(char*)*600);
    char*temp = (char*)malloc(sizeof(char*)*20);
    I'm willing to bet you're not allocating the amount of memory you think you are here. As someone else already pointed out, the proper way should be:
    Code:
    char *loadedfile = malloc(sizeof(*loadedfile)*600);
    char *temp = malloc(sizeof(*temp)*20);
    You should really read the comments people are posting here, because you keep making the same mistakes. Your segfault is most likely occuring because you are overwriting the loadedfile buffer with the line:
    Code:
    strcat(loadedfile,temp);
    You never check to see if you are adding more characters to loadedfile than there is room for.

  13. #13
    Registered User
    Join Date
    Sep 2005
    Posts
    15
    what? remember, total programing time < 20 hours, speak slo and use small words please.

    and wouldnt 599 slots be enough? i would have thought it would.

    and anyway, wouldnt that go wrong if i read a file with more than 599 ch? my file has 30 chars atm.
    Last edited by Ubber_C_Noob; 09-11-2005 at 01:16 AM.

  14. #14
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    A couple of comments
    1) The practice;

    Code:
       type *somename = (type *)malloc(sizeof(type *)*some_constant);
    is wrong. You've done this in a few places (with type as an int or as a char) Depending on the size of type vs size of a pointer to type, the code that uses "somename" may or may not give a buffer overrun. In C the typecast is optional (and viewed as bad style). In C++ it is mandatory. In C, use;
    Code:
       type *somename = malloc(sizeof(type)*some_constant);
    The latest version of ReadCatalogue is broken(). The following is it with comments added.
    Code:
    void ReadCatalogue(FILE *myfile)
    {
       char*loadedfile=(char*)malloc(sizeof(char*)*600);    /* refer note above */
       char*temp = (char*)malloc(sizeof(char*)*20);           /* refer note above */
       while(fgets(temp,sizeof(temp),myfile))                       /* sizeof(temp) will typically be 4  on 32 bit systems.  You probably want a value of about 19 */
       {
          strcat(loadedfile,temp);                                          /* undefined behaviour.   The buffer pointed at by loadedfile is not initialised */
       }
       printf(loadedfile);
    }

  15. #15
    Registered User
    Join Date
    Sep 2004
    Location
    California
    Posts
    3,268
    and wouldnt 599 slots be enough?
    First of all, you're not allocating 599 slots. You're allocating 600 * sizeof(char*) slots which probably ends up being 600 * 4 = 2400 depending on your machine.

    Second of all, how do you know the memory you've allocated is enough? You never check the file size beforehand, or keep track of how many bytes you are copying in to the loadedfile buffer.

    Here is a better version of your function
    Code:
    void ReadCatalogue(FILE *myfile)
    {
    	char		*loadedfile;
    	char		temp[512];
    	int		buffersize = 600;
    	int		bytescopied = 1; // start at 1 to account for null term
    
    	loadedfile = malloc(buffersize);
    	loadedfile[0] = '\0';  // so strcat() doesn't tank the first through the loop
    
    	while(fgets(temp,sizeof(temp),myfile))
    	{
    		bytescopied += strlen(temp);
    		if(bytescopied > buffersize)
    			break; // break from while loop if buffer isnt big enough to copy everything
    		strcat(loadedfile,temp);
    	}
    	printf(loadedfile);
    }

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Trouble with DMA Segmentation Faults
    By firestorm717 in forum C Programming
    Replies: 2
    Last Post: 05-07-2006, 09:20 PM
  2. oldiofclose.c and segmentation faults
    By sd_padilla in forum C Programming
    Replies: 1
    Last Post: 12-11-2005, 02:24 PM
  3. Segmentation faults on Linked Lists. (Please help!!)
    By summerrainx in forum C++ Programming
    Replies: 3
    Last Post: 03-19-2005, 07:23 AM
  4. segmentation faults pervade
    By ezwise in forum C Programming
    Replies: 8
    Last Post: 02-28-2005, 03:17 PM
  5. Locating A Segmentation Fault
    By Stack Overflow in forum C Programming
    Replies: 12
    Last Post: 12-14-2004, 01:33 PM