Contents of array changing within a struct in C

Question

I'm currently stumped on this problem I have.

I have this structure:

typedef struct corgi_entry {
  unsigned pup; //Supposed to be hex
} corgi_entry, *Pcorgi_entry;

typedef struct corgi {
  Pcorgi_entry *arrayCorgiHead;
} corgi, *Pcorgi;

I have this variables:

static corgi c1;

In an init function, I malloc my arrayCorgiHead

c1.arrayCorgiHead = (Pcorgi_entry *)malloc(sizeof(Pcorgi_entry) * corgiNumber));

The corgiNumber is just a placeholder, it can be however many, just the size of the index.

Given this, later on I'm running a loop. Every iteration of the loop, I'm given a corgi and an index from somewhere. If at that index it is null, I'm supposed to make a corgi and assign the pup data to be the same. If it's not null, I check the corgi I'm given with the corgi at the index to see if the pup match.

if(c1.arrayCorgiHead[index] == NULL){
    corgi_entry newcorgi_entry = {pupnumber};
    c1.arrayCorgiHead[index] = &newcorgi_entry;
    printf("Corgi Made!, pup: %10x\n", c1.arrayCorgiHead[index]->pup); //For debug purposes
}
else{ //Not null
    if(c1.arrayCorgiHead[index]->pup != given_corgi_pup){
        printf("Corgi Found?, pup: %10x\n", c1.arrayCorgiHead[index]->pup); //For debug purposes
        //Do stuff
    }
}

Here's the problem. In the file where I give this code the "external" index/corgis, on my third entry I give it the same as the first:

Index: 1 Corgi{pup = 123}
Index: 2 Corgi{pup = 456}
Index: 1 Corgi{pup = 123}

For some reason, when it processes the third entry, it says that it doesn't match. I print out the corgi pup at the array index, and I get some really weird number that doesn't make any sense (9814008 or something). This random number changes every time I ./corgisim and whatnot.

I do not touch the array at any other point besides what's written here. Why would my data be changing after a loop iteration?

*edit: To clarify my last point, my printf statements goes

Corgi Made!, pup: 123
Corgi Made!, pup: 456
Corgi Found?, pup: 20138940139 (random number that changes every time I run my program)

This code has been abbreviated for the sake of clarity (and corgi-ed, I don't know if that makes things clearer or not. The numbness in my brain has driven me insane). As you may be able to tell from the code, I'm not very familiar with C.

Thanks!

Edit 2: Thanks guys, solution worked like a charm! Pretty long question for a pretty simple problem, but I'm glad I figured it out. Much appreciated.

Oh dear, the error is you've allocated on the stack instead of the heap… — DarkDust
– DarkDust, Commented Apr 30, 2014 at 8:28
Could you explain further what you mean? I'm sorry, I'm not very proficient in C. Am I mismanaging memory? — user1657689
– user1657689, Commented Apr 30, 2014 at 8:33
BTW, I would avoid Pcorgi and Pcorgi_entry: it's tempting to write Pcorgi_entry foo = malloc(sizeof(Pcorgi_entry)); which is wrong (it would allocate the amount of memory needed for the pointer instead of the structure). I prefer plain corgi_entry *. But it's a matter of taste, so just take it as my opinion :-) — DarkDust
– DarkDust, Commented Apr 30, 2014 at 8:52
Alright, I'll make sure to do that. I don't need more small mistakes to drive me insane! — user1657689
– user1657689, Commented Apr 30, 2014 at 9:08

barak manos · Accepted Answer · 2014-04-30 08:34:59Z

1

Here is the problem:

corgi_entry newcorgi_entry = {pupnumber};
c1.arrayCorgiHead[index] = &newcorgi_entry;

Variable newcorgi_entry is a local variable, which is allocated on the stack. Therefore, once outside the variable's scope of declaration, you cannot rely on &newcorgi_entry as a valid memory address.

Here is the solution:

You should allocate the corgi_entry instance on the heap instead:

corgi_entry* newcorgi_entry = malloc(sizeof(corgi_entry));
newcorgi_entry->pup = pupnumber;
c1.arrayCorgiHead[index] = newcorgi_entry;

answered Apr 30, 2014 at 8:34

barak manos

30.3k10 gold badges67 silver badges117 bronze badges

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

1 Comment

user1657689 Over a year ago

I will give this a try! Before I do, is there a way to edit the already allocated memory for the arrayCorgiHead without allocating memory for each newcorgi_entry? My arrayCorgiHead when first init is supposed to all be null, but the memory is allocated for the amount of corgi_entry's that could possibly go into it right? Can I edit that memory, or do I have to allocate for a newcorgi_entry and then point my c1.arrayCorgiHead[index] to the memory address? (as you've shown me, thanks! :D)

DarkDust · Accepted Answer · 2014-04-30 08:44:08Z

The problem is how you "allocate" memory:

corgi_entry newcorgi_entry = {pupnumber};
c1.arrayCorgiHead[index] = &newcorgi_entry;

Here, newcorgi_entry is allocated on the stack. Then you assign the address of that stack variable to your array. Problem is, as soon as your function runs out of scope that memory is free to be used by any other function and sooner or later gets overwritten by some other code.

There are two solutions: either allocate on the heap using malloc, or don't use pointers at all. The first solution looks like this:

Pcorgi_entry newcorgi_entry = malloc(sizeof(corgi_entry));
newcorgi_entry->pub = pubnumber;
c1.arrayCorgiHead[index] = newcorgi_entry;

For the second solution there are a few variations. Here's one:

typedef struct corgi_entry {
  unsigned pup;
  int inUse;
} corgi_entry, *Pcorgi_entry;

typedef struct corgi {
  corgi_entry *arrayCorgiHead; // "Head" is the wrong name here, IMHO.
} corgi, *Pcorgi;


static corgi c1;

// calloc clears the memory to 0
c1.arrayCorgiHead = (corgi_entry *)calloc(corgiNumber, sizeof(corgi_entry));

if(!c1.arrayCorgiHead[index].inUse){
    c1.arrayCorgiHead[index].pub = pubNumber
    c1.arrayCorgiHead[index].inUse = 1;
    printf("Corgi Made!, pup: %10x\n", c1.arrayCorgiHead[index].pup); //For debug purposes
}
else{
    if(c1.arrayCorgiHead[index].pup != given_corgi_pup){
        printf("Corgi Found?, pup: %10x\n", c1.arrayCorgiHead[index].pup); //For debug purposes
        //Do stuff
    }
}

I think the corgi struct and its variable are unnecessary. I'd do it like this instead:

typedef struct corgi_entry {
  unsigned pup;
  int inUse;
} corgi_entry;

static corgi_entry *corgis;


// calloc clears the memory to 0
corgis = (corgi_entry *)calloc(corgiNumber, sizeof(corgi_entry));

if(!corgis[index].inUse){
    corgis[index].pub = pubNumber
    corgis[index].inUse = 1;
    printf("Corgi Made!, pup: %10x\n", corgis[index].pup); //For debug purposes
}
else{
    if(corgis[index].pup != given_corgi_pup){
        printf("Corgi Found?, pup: %10x\n", corgis[index].pup); //For debug purposes
        //Do stuff
    }
}

Yet another way would be if you define the "pubNumber" 0 to be illegal so you can us it as a sentinel instead:

typedef struct corgi_entry {
  unsigned pup;
} corgi_entry;

static corgi_entry *corgis;


// calloc clears the memory to 0
corgis = (corgi_entry *)calloc(corgiNumber, sizeof(corgi_entry));

// pub number 0 is illegal, so it must be an unused entry
if(corgis[index].pub == 0) {
    corgis[index].pub = pubNumber
    printf("Corgi Made!, pup: %10x\n", corgis[index].pup); //For debug purposes
}
else{
    if(corgis[index].pup != given_corgi_pup){
        printf("Corgi Found?, pup: %10x\n", corgis[index].pup); //For debug purposes
        //Do stuff
    }
}

Thank you for the very detailed answer! I will take this into consideration. And thank you for the alternative too. I will admit this is a small part of an assignment and pointers are required. I changed everything to corgis to capture the essence of my problem as opposed to posting everything about my assignment on here. The arrayCorgiHead is called that because the corgi_entry will later be linked lists, just didn't want to complicate things. But I agree if it was just like this, the head is kinda weird out of context. Thanks!!

Collectives™ on Stack Overflow

Contents of array changing within a struct in C

2 Answers 2

1 Comment

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related