Realloc a structure in c

Question

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

*cityList = realloc(*cityList - This is bad form. What happens in realloc fails — Ed Heal
– Ed Heal, Commented Dec 27, 2017 at 15:52
Questions seeking debugging help (why isn't this code working?) must include the desired behavior, a specific problem or error and the shortest code necessary to reproduce it in the question itself. Questions without a clear problem statement are not useful to other readers. See: How to create a minimal reproducible example. — Sourav Ghosh
– Sourav Ghosh, Commented Dec 27, 2017 at 15:53
Voting to close for not including all relevant code, especially the declaration of the cityList in main(). — underscore_d
– underscore_d, Commented Dec 27, 2017 at 16:16
What is N? You need to give an minimal reproducible example — Basile Starynkevitch
– Basile Starynkevitch, Commented Dec 27, 2017 at 16:19
Yes, please update your question with an example where all irrelevant code has been removed, but all relevant code (like the declaration of cityList in main, and anywhere you set *cityList) is present (and verify that you still see the problem). — Daniel H
– Daniel H, Commented Dec 27, 2017 at 16:20

Persixty · Accepted Answer · 2017-12-27 16:17:25Z

2

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.

edited Dec 27, 2017 at 16:17

answered Dec 27, 2017 at 16:11

Persixty

8,6193 gold badges15 silver badges38 bronze badges

Sign up to request clarification or add additional context in comments.

8 Comments

Paul Ogilvie Over a year ago

The de facto error return in C is 0 (FALSE). Only in exit is a non-zero value used as the failure exit of the program to the OS.

Persixty Over a year ago

@PaulOgilvie In all my experience return 0; is success (no error) and any non-zero value indicates an error. That allows error information to be encoded in the return value - if useful. The same rule applies to exit() for the same rationale. It makes sense if you see the return value as an error code as false as 'False. There was no error'.

Paul Ogilvie Over a year ago

Think of it as a bool, with 0 is false and non-zero is true. You may be using another convention, but that is not standard C in my experience. E.g. if (myFunc()) must now be coded as if (!myFunc()) which clearly reads oposite of the meaning.

Persixty Over a year ago

@PaulOgilvie I'm surprised. I've never seen widespread use of 0 as failure in C code. I know it's slightly counter intuitive but as I said it allows richer error information. In the example one number could be 'out of memory' and another 'file read error'. But it is only norm. Tastes vary.

alk Over a year ago

@PaulOgilvie: My bet goes on -1 to indicate an error in C. And: What is FALSE? ;-)

|

Paul Ogilvie · Accepted Answer · 2017-12-27 16:18:02Z

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.

Sagi Forbes · Accepted Answer · 2017-12-27 16:10:07Z

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)

dernst · Accepted Answer · 2017-12-28 14:49:49Z

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
    }
}

"Assuming you are wanting to simply reallocate an array of pointers" why? CITY * citylist would not allow doing so.
In C only values of void-pointers may be printed. u is for unsigned integers. To print a void-pointer use p and cast the pointer to void*.

Collectives™ on Stack Overflow

Realloc a structure in c

4 Answers 4

8 Comments

Comments

Comments

2 Comments

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

8 Comments

Comments

Comments

2 Comments

Related