CODEWITHSUNDEEP
cmemorystructdynamic-memory-allocationrealloc
Andrei Trasca
Andrei Trasca

Reputation: 39

Realloc a structure in c

So what I try to do is to dynamically allocate memory for a structure. The structure looks like this:

typedef struct{
   char name[N];
   int dimToys;
   TOY toys[N];
}CITY;

I read some input from a .txt file and copy it in my structure.

Then I have a function in which I would like to reallocate memory(and do some other irrelevant stuff) but my program works only when I use this function one single time. If I try to realloc the memory multiple times, then my program crashes.

It looks something like this:

void next_city(CITY** cityList, *dimCities, otherstuff){
    (*dimCities)++;
    *cityList = realloc(*cityList, (*dimCities) * sizeof(CITY*));
    otherstuff...
}

I tried running the debugger and it crashes at the line with realloc, when I call the function for the second time. The call of the function in the main function looks like this:

 cityList = malloc(sizeof(CITY*));
 for(...){
     ...
     next_city(&cityList, &dimCities, otherstuff);
 }

I've already tried using a temporary variable for the reallocation and then copy it in my original cityList, but it doesn't work neither.

LATER EDIT:

Because a lot of people told me to update my question with some clarifications, I'll just show my code more clear, after I made some modifications from what you all told me.

void next_city(char line[], CITY **cityList, int *dimCities){
(*dimCities)++;
*cityList = realloc(*cityList, (*dimCities) * sizeof(CITY)); //UPDATE CITYLIST DIMENSION
cityList[(*dimCities) - 1]->dimToys = 0;

char *word = strtok(line, " ");
strcpy((cityList[(*dimCities) - 1]->name), word); //INSERET NAME OF THE CITY
word = strtok(NULL, " ");
strcpy((cityList[(*dimCities) - 1]->toys[cityList[(*dimCities) - 1]->dimToys].toyType), word); //INSERT NAME OF THE TOY

}

int main(){
int nrPasi;
int dimCities = 0;
scanf("%d", &nrPasi);
fgetc(stdin);
int i;
CITY *cityList;
cityList = NULL;

char line[100];
for(i = 0;i < nrPasi;i++){
    fgets(line, 100, stdin);
    next_city1(line, &cityList, &dimCities);
}
return 0;

}

So I basically read something like

3 
Berlin car
Berlin doll 
Madrid jacket

And I just want to read it step by step. After I switched CITY* with CITY now realloc doesn't break my program, but it happens on the next line, when I try to access it. I get SISGEV

Upvotes: 2

Views: 2762

Answers (4)

dernst
dernst

Reputation: 72

Assuming you are wanting to simply reallocate an array of pointers (to CITY) then the following works when I tested it. But as someone mentioned above, perhaps you want to have an array of CITY then you would want to allocate memory for CITY as well. 

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

typedef struct{
    char name[10];
    int dimToys;
    int toys[10];
} CITY;

void next_city(CITY *** cityList, int *dimCities) {
    (*dimCities)++;
    *cityList = realloc(*cityList, (*dimCities) * sizeof(CITY*));
}

main(){
    CITY ** cityList;
    int dimCities=0;
    int i;

    cityList = malloc(sizeof(CITY*));
    printf("%p\n",  (void *) cityList); // print out the address of the memory allocated
    for (i = 0 ; i < 10; i++){
        next_city(&cityList, &dimCities);
        malloc(1);  // malloc more data to force some fragmenation so that you can see that realloc does something interesting.
        printf("%p\n",  cityList); // print out the address of the reallocated memory
    }
}

Upvotes: -1

Paul Ogilvie
Paul Ogilvie

Reputation: 25276

You reallocate sizeof(CITY*)

That should be sizeof(CITY), otherwise you allocate the size of a pointer.

That wouldn't be a problem if cityList is an array of poinetrs to CITYs, but from your code I do not see you next allocate memory for each CITY. So citylist is apparently an array of CITYs.

Because you assume realloc gave you storage for your struct, you started filling it with data. That will now overwrite the heap, causing your program to abort upon the next call to realloc because realloc now works on a corrupted heap.

Upvotes: 2

Persixty
Persixty

Reputation: 8579

You're making a common mistake of allocating space for a pointer to CITY not the structure itself. You need to allocate sizeof(CITY).

But there's a couple of other points to make...

cityList = NULL;//No need allocate anything up front.
dimCities=0u; //I'll assume this is size_t.
 for(...){
     ...
     if(next_city(&cityList, &dimCities /*, otherstuff*/)){
         //There was an error. What to do now?
     }
 }

And

int next_city(CITY** cityList, size_t *dimCities/*, otherstuff*/){
    (*dimCities)++;
    CITY* newList = realloc(*cityList, (*dimCities) * sizeof(CITY));
    if(newList==NULL){
        //out of memory..
        return 1;//Error return...
    }
    *cityList=newList;

    otherstuff...
    return 0;//Good return...
}

If you pass NULL to realloc it acts like malloc(). I don't know how you propose to handle errors but if there's insufficient memory to reallocate realloc() does nothing and returns NULL. Your way of allocating one object up front is fine. But personally this way is cleaner. Allocating at least one CITY may suit you because you never need to deal with cityList==NULL.

If this is a simple program you might just give up and exit() at this point. I've returned a error flag (a de facto C standard) so the surrounding program can handle it.

Upvotes: 2

Sagi Forbes
Sagi Forbes

Reputation: 2197

In the realloc, you use sizeof(CITY*). It seems to me that you tried to get the size of the CITY struct. So your code does not do that.

The size of CITY* is the size of the pointer holding the CITY struct, which is based on your system architecture (64 or 32 bit). This way the content of CITY cannot pass between the reallocation, causing a memory buffer override and crashing the application.

If you need the size of the CITY struct just use sizeof(CITY)

Upvotes: 1

Related Questions