Dynamic array won't work

[Unregistered]

I know I've made an error somewhere that is causing my resizable array to only have one element and nothing else, but I can't find it and my compiler's debug function doesn't work (school computer, go figure).
Here's the resizing function and the function that calls it, ignore the C++ input and output calls, that's the only thing C++ in this program.

int main(void)
{
  int sel = 0, count = 1;
  long key = 0;
  struct Employee *emp;
  //old compiler, it makes me cast malloc
  emp = (struct Employee *)malloc(sizeof *emp);

  if(emp == NULL)
    return EXIT_FAILURE;
  else{
    sel = menu();
    while(sel != 6){
      if(sel == 1)
	makeNew(emp, &count);
      else if(sel == 6)
	break;
      else
	cout<<"ERROR: Invalid input"<<flush;
      sel = menu();
    }
  }
  free(emp);
  return EXIT_SUCCESS;
}

int makeNew(struct Employee *empArray, int *size)
{
  int count = *size;
  struct Employee *temp;
  //ditto, my old compiler makes me cast realloc as well
  temp = (struct Employee *)realloc(empArray, ++count * sizeof *empArray);
  if(temp == NULL)
    return EXIT_FAILURE;
  *size++;
  empArray = temp;
  //Information for the new element is entered here

  free(temp);
  return EXIT_SUCCESS;
}

The first time I call the makeNew function, I actually do reallocate it but the size stays the same when I loop through the array and try to print everything. The second time I call the makeNew function it returns fail.

[Shiro]

>emp = (struct Employee *)malloc(sizeof *emp);

This is not right. Now you're allocating space for a pointer, but you wanted space for a whole structure:

emp = (struct Employee *) malloc (sizeof (struct Employee));

>cout<<"ERROR: Invalid input"<<flush;

Hmmm. You're at the C board, not at the C++ board. :-)

>temp = (struct Employee *)realloc(empArray, ++count * sizeof *empArray);

The variable count is not really needed. You could also use

(*size +1)

This will not change *size.

temp = (struct Employee *) realloc (empArray, (*size + 1) * sizeof (struct Employee));

I have not compiled this, but I these were the things I noted. Hope it helps you some.

[Unregistered]

>This is not right. Now you're allocating space for a pointer, but you wanted space for a whole structure:

The pointer is was declared as a structure pointer and dereferenced, this should work unless everyone on comp.lang.c is wrong. This apparently works fine btw, it's when I try to resize this pointer to two instead of one that I get problems

>Hmmm. You're at the C board, not at the C++ board. :-)

I know that, but I use malloc and realloc and feel that C programmers would be better suited to answer my question that programmers who are used to new and delete.

>The variable count is not really needed.

I tried that and decided against it for some reason, I'll remember eventually why I did that.

[Shiro]

>emp = (struct Employee *)malloc(sizeof *emp);

According to the standard it is valid. So you're right.

[Deckard]

emp is a variable containing a memory address (fondly known as a pointer). When makeNew() is called, a copy of the address is passed in as empArray. When you reassign empArray to point to temp, you are changing the memory address within your local copy of the variable, but it is not changing the address stored in emp back in main. That is why emp is not resized in the first attempt. You can overcome this by passing emp as a pointer to a pointer.

(if I could splel I wouldn't have to edit)

[Shiro]

What about the free(temp) in makeNew? It frees the memory where temp is pointing to. And empArray is pointing to this.

[Sorensen]

Also you may want to pass in a pointer to a pointer, empArray is local to the function.

[Unregistered]

>What about the free(temp) in makeNew?

After checking that temp actually has something in it (aside from null), I assign the address that temp is pointing to to empArray. Unless I'm mistaken I can then free the memory that temp takes up because empArray still has everything. If I am mistaken let me know, but I will try and see what happens when I remove free.

>empArray is local to the function.

I did try using a pointer to a pointer, could you explain why that would work better than what I have? Assuming I can get it to work

[Unregistered]

I remember why I didn't use a pointer to a pointer for empArray. Either when I tried to realloc nothing would happen or if I dereferenced empArray in the realloc call I would get a general protection fault when I tried to run the function.

[Deckard]

An alternative to passing in a pointer to a pointer is to return a pointer to a Employee struct. In makeNew(), you would return (temp);, and your call to makeNew() from main would be:

emp = makeNew( emp, &count );

[Deckard]

...er, just be careful to take care of the allocated memory that emp used to point to.

[Shiro]

In main() you have a pointer to the structure. This pointer is passed to makeNew(). After reallocating, you let empArray point to temp. So empArray and temp are both pointing to the same memory block. Then you free temp, then this has the same result for empArray. I tested this with a little program:

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

void fill_a (int *a)
{
int *b;

b = (int *) realloc (a, 3);

b [0] = 1;
b [1] = 2;
b [2] = 3;

a = b;
}


int main (void)
{
int *a;

a = (int *) malloc (2);

fill_a (a);

printf ("%d %d %d\n", a [0], a[1], a[2]);

free (a);

return 1;
}

Since a is pointing to the same space as allocated for b, the free(a) in main() implies freeing the memory block of b.

[Salem]

> //old compiler, it makes me cast malloc
> emp = (struct Employee *)malloc(sizeof *emp);
No, the cast is necessary because you're compiling this as C++ and not C.

OTOH, it could be a really old compiler which returns char* instead of void*, in which case, its time you upgraded to something which is ANSI compatible.

> int makeNew(struct Employee *empArray, int *size)
Since you ignore the return result, it's pointless returning SUCCESS/FAIL.

This would be better

struct Employee *makeNew(struct Employee *empArray, int *size)
{
  int count = *size;
  struct Employee *temp;
  temp = (struct Employee *)realloc(empArray, ++count * sizeof *empArray);
  if(temp == NULL) {
      // no room for more, but what's there is still OK
      // warn user that no more additions are possible
      // and return with the existing data
      return empArray;
  }

  // *size++;
  // This increments the pointer, not what it points at
  (*size)++;

  empArray = temp;

  //Information for the new element is entered here

  // free(temp);
  // NO!
  // you lose all the memory by doing this

  return empArray;
}

You call this new function like so
&nbsp; emp = makeNew(emp, &count);

You can start off with
&nbsp; count = 0;
&nbsp; emp = NULL;
and it would work just as well

[Unregistered]

If you assign the newly allocated memory to emp by returning temp from makeNew then there shouldn't be a problem with where emp used to point. It still has the same values except the array is one element larger. What would I do about temp then? It still has memory allocated for it and I can't free it before returning because temp is returned.

My original question still stands btw, how do I get this program to reallocate another element for the array?

[Shiro]

>What would I do about temp then? It still has memory allocated >for it and I can't free it before returning because temp is >returned.

Pointer emp is pointing to temp. So you don't need to worry about temp.