Thread: [C] Struct allocation crash?

  1. #1
    Registered User
    Join Date
    Feb 2009
    Posts
    12

    [C] Struct allocation crash?

    (Dev-C++ in Windows Vista / GCC in Ubuntu)

    My goal is to eventually read line by line from a file that contains combined lengths of text like those joined into the "input" variable.
    Currently, the code below crashes the console in windows and in linux it points to a "realloc(): invalid next size" error.

    I realize that most of this code is poor programming style, but I'm only looking for it to work at this point. I was wondering if there was a much more efficient way to do this code. This would probably be much easier if the number of records and the number of subnames were constant, like they are in the example "input" variable.

    So I guess my questions are, what is causing the code to crash and is there a more efficient way to store an unknown number of records containing an unknown number of subnames in each record?

    Please ask questions if I left out something important or need to clarify anything. Thanks!

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    typedef struct {
    	int ID;
    	char* name;
    	int subs;
    	char** subname;
    } RECORD;
    
    int main() {
    	/*Sample of what would be read from a text file*/
    	char input[2][80] = {"1,Entry,ProcStart,ProcLoop,ProcFunc,ProcEnd,ProcStop",
    		"2,Phase,ProcBegin,ProcSpin,ProcHelp,ProcEnd,ProcHalt"};
    	RECORD* record = (RECORD*) malloc(sizeof(RECORD));;
    	if(record == NULL) return -1;
    	int records = 0;
    	int loop;
    	for(loop=0;loop<2;loop++) {
    		record = (RECORD*) realloc(record, sizeof(RECORD)*(records+1));
    		if(record == NULL) return -1;
    		record[records].ID = atoi(strtok(input[loop],","));
    		char* temp = strtok(NULL,",");
    		record[records].name = (char*) malloc(sizeof(char)*(strlen(temp)+1));
    		if(record[records].name == NULL) return -1;
    		strcpy(record[records].name,temp);
    		record[records].subs = 0;
    		int i;
    		while((temp=strtok(NULL,","))!=NULL) {
    			i = record[records].subs;
    			record[records].subname = (char**) realloc(record[records].subname,i+1);
    			if(record[records].subname == NULL) return -1;
    			record[records].subname[i] = (char*) malloc(sizeof(char)*(strlen(temp)+1));
    			if(record[records].subname[i] == NULL) return -1;
    			strcpy(record[records].subname[i],temp);
    			record[records].subs++;
    		}
    		records++;
    	}
    
    	/*Test the resulting data to see if the variables are stored correctly*/
    	int x,y;
    	for(y=0;y<2;y++) {
    		printf("%d:\n",y);
    		for(x=0;x<record[y].subs;x++) {
    			printf("%s\n",record[y].subname[x]);
    		}
    	}
    	getchar(); /*Stops the window from closing in the Windows console*/
    	return 0;
    }

  2. #2
    Registered User
    Join Date
    Sep 2007
    Posts
    1,012
    Code:
    record[records].subname = (char**) realloc(record[records].subname,i+1);
    You forgot to multiply the size by sizeof(char*) here; so (assuming pointers are 4 bytes) you're allocating 1/4 the needed space.

    Funny enough, your multiplication by sizeof(char) in the other allocations is not necessary, since sizeof(char) is always 1. It doesn't hurt to do the multiplication, but I find it cleaner to leave it out.

  3. #3
    spurious conceit MK27's Avatar
    Join Date
    Jul 2008
    Location
    segmentation fault
    Posts
    8,300
    Also: you should use malloc there and not realloc. Your pointer has a size of 4 -- you are not changing that. You are allocating space that the pointer points to. So you don't (and can't) change the size of the pointer, you give it somewhere to point to of whatever size. Only use realloc on a variable on which you have already used malloc.
    C programming resources:
    GNU C Function and Macro Index -- glibc reference manual
    The C Book -- nice online learner guide
    Current ISO draft standard
    CCAN -- new CPAN like open source library repository
    3 (different) GNU debugger tutorials: #1 -- #2 -- #3
    cpwiki -- our wiki on sourceforge

  4. #4
    Registered User
    Join Date
    Dec 2006
    Location
    Canada
    Posts
    3,229
    Code:
    record[records].subname
    is not previously malloc'd.

    For memory-related problems, try valgrind.
    http://valgrind.org/

  5. #5
    Registered User
    Join Date
    Feb 2009
    Posts
    12
    Thanks for your help everyone. I was not aware that I needed a sizeof(char*). I'll look that up later to learn why. I also added the malloc before the while loop to stop the "realloc()" error.

    I did this for both my windows version and the linux version (same code, Ubuntu is on a virtual machine). The windows one continues to crash as soon as it is run for some reason. The linux one works perfectly, though.

    On an unrelated note, every time I try to download Valgrind into my Ubuntu virtual machine, it makes my whole computer Blue Screen and restart. Definitely annoying on all hairs.

    I have another question:
    You may have noticed that my code does NOT include "free(...)" statements. I am relying on the OS to free that memory at the end of the application, since I will be needing to access the data throughout the life of the application. Is this acceptable, despite being against good programming? I figured it would be quite annoying to free all of the data manually, especially since I will later have many different kinds of structs each holding varying sizes of information.

  6. #6
    Registered User
    Join Date
    Dec 2006
    Location
    Canada
    Posts
    3,229
    Blue screen? kernel panic is black last time I checked...

    Oh, you mean the Windows host.

    Hardware problem perhaps? or maybe driver.

    Code:
    sudo apt-get install valgrind
    in the virtual machine should do.

    You may have noticed that my code does NOT include "free(...)" statements. I am relying on the OS to free that memory at the end of the application, since I will be needing to access the data throughout the life of the application. Is this acceptable, despite being against good programming? I figured it would be quite annoying to free all of the data manually, especially since I will later have many different kinds of structs each holding varying sizes of information.
    Bad practice, but should work.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Assignment HELP!!
    By cprogrammer22 in forum C Programming
    Replies: 35
    Last Post: 01-24-2009, 02:24 PM
  2. Looking for constructive criticism
    By wd_kendrick in forum C Programming
    Replies: 16
    Last Post: 05-28-2008, 09:42 AM
  3. Concatenating in linked list
    By drater in forum C Programming
    Replies: 12
    Last Post: 05-02-2008, 11:10 PM
  4. Dynamic allocation (I thought it would crash)
    By Baaaah! in forum C Programming
    Replies: 16
    Last Post: 11-30-2005, 05:10 PM
  5. Search Engine - Binary Search Tree
    By Gecko2099 in forum C Programming
    Replies: 9
    Last Post: 04-17-2005, 02:56 PM