0

UPDATE: I think I've answered my own question, except for some possible issues with memory leaks.

ORIGINAL QUESTION HERE, ANSWER BELOW.

Background: I'm doing some numerical computing, but I almost never use languages that require me to manage memory on my own. I'm piping something out to C now and am having trouble with (I think) pointer reference issues.

I have two arrays of doubles that are growing in a while loop, and at each iteration, I want to free the memory for the smaller, older array, set 'old' to point to the newer array, and then set 'new' to point to a larger block of memory.

After looking around a bit, it seemed as though I should be using pointers to pointers, so I've tried this, but am running into "lvalue required as unary ‘&’ operand" errors.

I start with:

double ** oldarray;
oldarray = &malloc(1*sizeof(double));
double ** newarray;
newarray = &malloc(2*sizeof(double));

These initializations give me an "lvalue required as unary ‘&’ operand" error, and I'm not sure whether I should replace it with

*oldarray = (double *) malloc(1*sizeof(double));

When I do that, I can compile a simple program (It just has the lines I have above and returns 0) but I get a seg fault.

The rest of the program is as follows:

while ( <some condition> ) {

// Do a lot of processing, most of which is updating
// the values in newarray using old array and other newarray values. 

// Now I'm exiting the loop, and growing and reset ing arrays.
free(*oldarray)        // I want to free the memory used by the smaller, older array.
*oldarray = *newarray  // Then I want oldarray to point to the larger, newer array.
newarray = &malloc( <previous size + 1>*sizeof(double))
}

So I'd like to be, at each iteration, updating an array of size (n) using itself and an older array of size (n-1). Then I want to free up the memory of the array of size (n-1), set 'oldarray' to point to the array I just created, and then set 'newarray' to point to a new block of size (n+1) doubles.

Do I actually need to be using pointers to pointers? I think my main issue is that, when I set old to new, they share a pointee, and I then don't know how to set new to a new array. I think that using pointers to pointers gets me out of this, but, I'm not sure, and I still have the lvalue errors with pointers to pointers.

I've checked out C dynamically growing array and a few other stack questions, and have been googling pointers, malloc, and copying for about half a day.

Thanks!

HERE IS MY OWN ANSWER

I've now got a working solution. My only worry is that it might contain some memory leaks.

Using realloc() works, and I also need to be careful to make sure I'm only free()ing pointers that I initialized using malloc or realloc, and not pointers initialized with double * oldarray;.

The working version goes like this:

double * olddiagonal = (double *) malloc(sizeof(double));
olddiagonal[0] = otherfunction(otherstuff);
int iter = 1;

// A bunch of other stuff
while (<error tolerance condition>) {

  double * new diagonal = (double *) malloc((iter+1)*sizeof(double));
  newdiagonal[0] = otherfunction(moreotherstuff);

  // Then I do a bunch of things and fill in the values of new diagonal using
  // its other values and the values in olddiagonal.

  // To finish, I free the old stuff, and use realloc to point old to new.    
  free(olddiagonal);
  olddiagonal = (double *) realloc(newdiagonal, sizeof(double) * (iter+1));

  iter++
}

This seems to work for my purposes. My only concern is possible memory leaks, but for now, it's behaving well and getting the correct values.

Community
  • 1
  • 1
  • Have you done C programming befoere? There's a bit of bad mojo here. For example, `oldarray = &malloc(1*sizeof(double));` is completely incorrect in syntax. `malloc` is a function call. Think about what `oldarray` represents. It's a pointer to a list of pointers. So you want to give it memory to hold however many pointers you want to maintain. That would mean `oldarray = malloc(N*sizeof(double *))` where `N` is however many pointers you need. – lurker Mar 11 '14 at 18:03
  • What about for the `double ** old; *old = malloc(n*sizeof(double));` version? – user2608609 Mar 11 '14 at 18:11
  • Note also: `malloc(1 * sizeof(double))` allocates enough space for one `double` value. You want to allocate space for many pointers to doubles. Thus, you get a segfault. You might want to use `realloc` as well, if you want to keep bumping up the memory. – lurker Mar 11 '14 at 18:13
  • Are you looking for `realloc()` ? http://www.cplusplus.com/reference/cstdlib/realloc/ To use it, do something like `double* array=NULL;double* newptr; newptr=realloc(array,sizeof(double)*42);if(newptr!=NULL){array=newptr;}else{printf("realloc failed\n")};` – francis Mar 11 '14 at 18:52
  • Beware: you're on your way to becoming a [Three Star Programmer](http://c2.com/cgi/wiki?ThreeStarProgrammer). – pmg Mar 11 '14 at 22:17
  • Really? My last answer doesn't use any pointers to pointers. – user2608609 Mar 11 '14 at 22:29

1 Answers1

0

Here are some explanations:


double ** oldarray;
oldarray = &malloc(1*sizeof(double));

is wrong, because you don't store the result of malloc() anywhere, and since it is not stored anywhere, you can't take its address. You can get the effect that you seem to have had in mind by adding an intermediate variable:

double* intermediatePointer;
double** oldarray = &intermediatePointer;
intermediatePointer = malloc(1*sizeof(*intermediatePointer);

oldarray is now a pointer to the memory location of intermediatePointer, which points to the allocated memory slap in turn.


*oldarray = (double *) malloc(1*sizeof(double));

is wrong, because you are dereferencing an unitialized pointer. When you declare oldarray with double** oldarray;, you are only reserving memory for one pointer, not for anything the pointer is supposed to point to (the memory reservation is independent of what the pointer points to!). The value that you find in that pointer variable is undefined, so you have absolutely no control about what memory address you are writing to when you assign something to *oldarray.

Whenever you declare a pointer, you must initialize the pointer before you dereference it:

int* foo;
*foo = 7;    //This is always a bug.

int bar;
int* baz = &bar;   //Make sure the pointer points to something sensible.
*baz = 7;    //OK.

Your answer code is indeed correct. However, it can be improved concerning style:

The combination of

int iter = 1;
while (<error tolerance condition>) {
    ...
    iter++
}

calls for the use of the for() loop, which encapsulates the definition and incrementation of the loop variable into the loop control statement:

for(int iter = 1; <error tolerance condition>; iter++) {
    ...
}

In C, the cast of the return value of malloc() is entirely superfluous, it only clutters your code. (Note however that C++ does not allow the implicit conversion of void*s as C does, so int *foo = malloc(sizeof(*foo)) is perfectly valid C, but not legal in C++. However, in C++ you wouldn't be using malloc() in the first place.)

cmaster - reinstate monica
  • 38,891
  • 9
  • 62
  • 106