call to realloc() inside a function: memory problem

simone.marras · 11-30-2008, 07:58 AM

Hi there, I am writing a code composed of a main function calling another function (called CSR in the wrapped code) whose arguments are pointers to 1D arrays.
The pointers are to be modified in length inside the function by means of realloc(), and returned to the main program with the new size and new elements.
The problem is that, although the function fills the arrays correctly ( I verify this by printing the array inside the function), such arrays are not passed back to the main correctly, in fact if I print the array from inside main(), the elements are not correct. It seems, from the output, that the memory allocated for the arrays is freed before the arrays are returned to main.

Array Jc and Ac should look like (I only put the first 10 elements of the total 57):
Jc = 1, 2, 4, 1, 2, 3, 4, 5, 2, 3, 5, 6, ...
Ac = 101, 102, 103, 104, 105, 106, ...

but when printed from inside main(), I get the following instead:
Jc = 134524928, 2, 4, 57, 1, 3, 9, 13, ...
Ac = 101, 0, 0, 0, 0, 0, 0, ....

I wrap my code here, I hope someone can help
Thanks
S.

Main():

Code:

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

#include"nrutil.h"
#include"CSR.h"

main()
{
	int i,j,k, nz=0;

	double A[][12]={{101, 102, 0, 103, 0, 0, 0, 0, 0, 0, 0, 0},{104, 105, 106, 107, 108, 0, 0, 0, 0, 0, 0, 0},{0,109,110,0,111,112,0,0,0,0,0,0},{113,114,0,115,116,0,117,0,0,0,0,0},{0,118,119,120,121,122,123,124,0,0,0,0},{0,0,125,0,126,127,0,128,129,0,0,0},{0,0,0,130,131,0,132,133,0,134,0,0},{0,0,0,0,135,136,137,138,139,140,141,0},{0,0,0,0,0,142,0,143,144,0,145,146},{0,0,0,0,0,0,147,148,0,149,150,0},{0,0,0,0,0,0,0,151,152,153,154,155},{0,0,0,0,0,0,0,0,156,0,157,158}};
	
	
	double *Ac;
	int *Ic, *Jc;

	/*Allocate dynamic memory */
		Ac = malloc((size_t) ((nz+1)*sizeof(double)) );
		Jc = malloc((size_t) ((nz+1)*sizeof(int)) );
		Ic = malloc((size_t) ((13)*sizeof(int)) );
		/*Jc = ivector(1,nz+1);

	/*Call CSR() */
		CSR(A, 12, 12, &nz, Ac, Ic, Jc, 0);
		
	
		for(i=0; i<=nz-1; i++)
			printf("Ac[%d] = %.1f\t Jc[%d] = %d\n", i, Ac[i], i, Jc[i]);
		
		for(i=0; i<=12; i++)
			printf("Ic[%d] = %d\n", i, Ic[i]);		
		
		
	/*Free dynamic memory*/
	free(Ac);
	free(Jc);
	free(Ic);
	
return;
}

CSR()

Code:

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

void CSR(double A[][12], int nrow, int ncol, int *nz, double *Ac, int *Ic, int *Jc, int array_numb)
{
 /*	array_numb:put 0 if your array is allocated in a 0-type style (n elementes from 0 to n-1)
  *			    put 1 if your array is allocated in a 1-type style (n elementes from 1 to n)
  */

	int i,j,k;
	int _nz;
	
	/*count nz: nonzero elements*/
	_nz = 0;
		for(i=array_numb; i<=nrow-1+array_numb; i++){
			for(j=array_numb; j<=ncol-1+array_numb; j++){
				if( A[i][j] != 0.0 ){
					_nz++;
					Ac = realloc(Ac, (size_t) _nz * sizeof(double));
					Jc = realloc(Jc, (size_t) _nz * sizeof(int));
					
					Ac[_nz-1] = A[i][j];
					Jc[_nz-1] = j+1;
					
					Ic[i+1] = _nz+1;
					
				}
				
				
			}
		}	
		//This goes here because of the point where _nz is updated inside the loop:
		Ic[0] = Jc[0];
		
		
		for(i=0; i<=_nz-1; i++)
			printf("Ac[%d] = %.1f\t Jc[%d] = %d\n", i, Ac[i], i, Jc[i]);
		
		for(i=0; i<=12; i++)
			printf("Ic[%d] = %d\n", i, Ic[i]);
		
		*nz = _nz;
	
return;
}

anon · 11-30-2008, 08:02 AM

If you want to assign a new value to the pointer in a function (and not to what is pointed to), you have to pass a pointer to this pointer, as otherwise you'd be updating a copy.

Also you shouldn't assign the result of realloc directly back to the same pointer. If it returns NULL, you'll have lost the pointer and leaked memory.

MK27 · 11-30-2008, 08:05 AM

I don't have "nrutil.h" so I can't try this, but you are passing values (not pointers) to a function that doesn't return anything. If you want to change the values in place, you should pass your function pointers.

What do your variables look like before you call CSR? Are they correct? Are they the same inside main as they were before? If so, this is definately the problem.

ps. regarding anon's problem about realloc: if realloc returns a NULL pointer everything is pretty much all over anyway, so you can just go:

if ((ptr=realloc(ptr,size))==NULL) fatal("realloc fail");

simone.marras · 11-30-2008, 08:09 AM

Hi Anon, thanks for replying so quickly. I am not sure on what you are suggesting: if I want to fill in a previously empty array Jc of defined of type int *Jc in main(), and passed to CRS as CRS(Jc), why does it not work that way? I mean, why Jc is filled in correctly inside CRS, but when I print it back from main(), I get a different array?

From the code that I wrapped, what part is the one that is causing memory leak?
Thanks a lot again
Simone

**laserlight** · 11-30-2008, 08:18 AM

Quote:

Originally Posted by simone.marras

I mean, why Jc is filled in correctly inside CRS, but when I print it back from main(), I get a different array?

The same reason why when you pass say, an int to a function and assign it a value, the int that you had in the caller of that function does not change. You passed the pointer by value, so the change to the pointer in the called function merely changes the local copy of the pointer. You need to pass a pointer to a pointer.

simone.marras · 11-30-2008, 08:19 AM

Hi there MK7, thanks for replying:

Quote:

Originally Posted by MK27

I don't have "nrutil.h" so I can't try this, but you are passing values (not pointers) to a function that doesn't return anything. If you want to change the values in place, you should pass your function pointers.[/b]

about nrutil.h you shouldn't worry; I forgot to erase the line from the wrapped code, but there is no call to the nrutil.h functions in this code. I am only using stdlib.h calls

Quote:

Originally Posted by MK27

What do your variables look like before you call CSR? Are they correct? Are they the same inside main as they were before? If so, this is definately the problem.
[/b]

The pointers Jc and Ac in main are defined as arrays of int and double respectively, and they are of size 1 when firstly assigned in main. The need for CSR is due to the fact that I need to count the nonzero elements of the matrix A before knowing how many elements Ac and Jc will have. From here the need for realloc.
So, before I call CSR, Ac and Jc have only 1 element each, and the value is zero.
The I call CSR inside which, by means of realloc, I increase their size with the increasing value of a counter defined in CSR itself, and start filling them up with the needed elements.
Within CSR Ac and Jc, if printed to screen, are being correctly filled, but when I print them from main once returned there, their elements are just gone.

Quote:

Originally Posted by MK27

ps. regarding anon's problem about realloc: if realloc returns a NULL pointer everything is pretty much all over anyway, so you can just go:

if ((ptr=realloc(ptr,size))==NULL) fatal("realloc fail");

**laserlight** · 11-30-2008, 08:23 AM

Quote:

Originally Posted by simone.marras

The pointers Jc and Ac in main are defined as arrays of int and double respectively, and they are of size 1 when firstly assigned in main. The need for CSR is due to the fact that I need to count the nonzero elements of the matrix A before knowing how many elements Ac and Jc will have. From here the need for realloc.

It might be better to write a function to perform that count. Call that function, and only then do you malloc() for the Jc and Ac arrays. At this point, your initial malloc() is wasted since you are going to realloc() with the correct sizes anyway. More generally, try to declare/create variables near first use (though this is somewhat harder in C, as per the C90 standard, than with the C99 standard or C++).

simone.marras · 11-30-2008, 08:30 AM

Quote:

Originally Posted by laserlight

It might be better to write a function to perform that count. Call that function, and only then do you malloc() for the Jc and Ac arrays. At this point, your initial malloc() is wasted since you are going to realloc() with the correct sizes anyway. More generally, try to declare/create variables near first use (though this is somewhat harder in C, as per the C90 standard, than with the C99 standard or C++).

HI Laserlight, thanks a lot for you reply. I was thinking of doing that first, but I wanted to avoid to have too many "for" loops and especially, I'd like to have only one function that does it all and in order to keep the function more usable.

I may go for your suggestion for the time being so that I can proceed with my code, but if any suggestion on what is causing the current error comes out, I'd be happy to know about it and solve it

Thank you
Simone

**laserlight** · 11-30-2008, 08:36 AM

Quote:

Originally Posted by simone.marras

HI Laserlight, thanks a lot for you reply. I was thinking of doing that first, but I wanted to avoid to have too many "for" loops and especially, I'd like to have only one function that does it all and in order to keep the function more usable.

A function should do one thing and do it well. You could have a function that does a series of things as one unit, but that function would call other functions that each do a particular thing in the series.

Quote:

Originally Posted by simone.marras

I may go for your suggestion for the time being so that I can proceed with my code, but if any suggestion on what is causing the current error comes out, I'd be happy to know about it and solve it

I already addressed this in post #5.

MK27 · 11-30-2008, 09:27 AM

Quote:

Originally Posted by laserlight

I already addressed this in post #5.

I think what laserlight means is what the rest of us are saying, which you are not passing and creating Ac, Ic, and Jc properly. This will at least make the arrays identical in main:

Code:

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

#include"CSR.h"

main()
{
	int i,j,k, nz=0;

	double A[][12]={{101, 102, 0, 103, 0, 0, 0, 0, 0, 0, 0, 0},{104, 105, 106, 107, 108, 0, 0, 0, 0, 0, 0, 0},{0,109,110,0,111,112,0,0,0,0,0,0},{113,114,0,115,116,0,117,0,0,0,0,0},{0,118,119,120,121,122,123,124,0,0,0,0},{0,0,125,0,126,127,0,128,129,0,0,0},{0,0,0,130,131,0,132,133,0,134,0,0},{0,0,0,0,135,136,137,138,139,140,141,0},{0,0,0,0,0,142,0,143,144,0,145,146},{0,0,0,0,0,0,147,148,0,149,150,0},{0,0,0,0,0,0,0,151,152,153,154,155},{0,0,0,0,0,0,0,0,156,0,157,158}};
	
	
	double Ac[58];
	int Ic[13], Jc[58];


	/*Call CSR() */
		CSR(A, 12, 12, &nz, Ac, Ic, Jc, 0);
		
	
		for(i=0; i<=nz-1; i++)
			printf("Ac[%d] = %.1f\t Jc[%d] = %d\n", i, Ac[i], i, Jc[i]);
		
		for(i=0; i<=12; i++)
			printf("Ic[%d] = %d\n", i, Ic[i]);		
		
	
return;
}

and in CSR remove the realloc code:

Code:

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

void CSR(double A[][12], int nrow, int ncol, int *nz, double *Ac, int *Ic, int *Jc, int array_numb)
{
 /*	array_numb:put 0 if your array is allocated in a 0-type style (n elementes from 0 to n-1)
  *			    put 1 if your array is allocated in a 1-type style (n elementes from 1 to n)
  */

	int i,j,k;
	int _nz;
	
	/*count nz: nonzero elements*/
	_nz = 0;
		for(i=array_numb; i<=nrow-1+array_numb; i++){
			for(j=array_numb; j<=ncol-1+array_numb; j++){
				if( A[i][j] != 0.0 ){
					_nz++;
					Ac[_nz-1] = A[i][j];
					Jc[_nz-1] = j+1;
					
					Ic[i+1] = _nz+1;
					
				}
				
				
			}
		}	
		//This goes here because of the point where _nz is updated inside the loop:
		Ic[0] = Jc[0];
		
		
		for(i=0; i<=_nz-1; i++)
			printf("Ac[%d] = %.1f\t Jc[%d] = %d\n", i, Ac[i], i, Jc[i]);
		
		for(i=0; i<=12; i++)
			printf("Ic[%d] = %d\n", i, Ic[i]);
		
		*nz = _nz;
	
return;
}

MK27 · 11-30-2008, 09:34 AM

Your original dynamic allocation will work if you make those variables global (I combined the two files here):

Code:

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

void CSR(double A[][12], int nrow, int ncol, int *nz, int array_numb);

double *Ac;
int *Ic, *Jc;

main()
{
	int i,j,k, nz=0;

	double A[][12]={{101, 102, 0, 103, 0, 0, 0, 0, 0, 0, 0, 0},{104, 105, 106, 107, 108, 0, 0, 0, 0, 0, 0, 0},{0,109,110,0,111,112,0,0,0,0,0,0},{113,114,0,115,116,0,117,0,0,0,0,0},{0,118,119,120,121,122,123,124,0,0,0,0},{0,0,125,0,126,127,0,128,129,0,0,0},{0,0,0,130,131,0,132,133,0,134,0,0},{0,0,0,0,135,136,137,138,139,140,141,0},{0,0,0,0,0,142,0,143,144,0,145,146},{0,0,0,0,0,0,147,148,0,149,150,0},{0,0,0,0,0,0,0,151,152,153,154,155},{0,0,0,0,0,0,0,0,156,0,157,158}};
	
	

	/*Allocate dynamic memory */
		Ac = malloc((size_t) ((nz+1)*sizeof(double)) );
		Jc = malloc((size_t) ((nz+1)*sizeof(int)) );
		Ic = malloc((size_t) ((13)*sizeof(int)) );
		/*Jc = ivector(1,nz+1);

	/*Call CSR() */
		CSR(A, 12, 12, &nz, 0);
		
	
		for(i=0; i<=nz-1; i++)
			printf("Ac[%d] = %.1f\t Jc[%d] = %d\n", i, Ac[i], i, Jc[i]);
		
		for(i=0; i<=12; i++)
			printf("Ic[%d] = %d\n", i, Ic[i]);		
		
		
	/*Free dynamic memory*/
	free(Ac);
	free(Jc);
	free(Ic);
	
return;
}


void CSR(double A[][12], int nrow, int ncol, int *nz, int array_numb)
{
[...remains the same...]

Witness CSR only requires 5 arguments now.

**laserlight** · 11-30-2008, 09:45 AM

Quote:

Originally Posted by MK27

Your original dynamic allocation will work if you make those variables global

How would you justify the use of global variables, i.e., what convinces you that Ac, Ic, and Jc are best declared as global variables?

Quote:

Originally Posted by MK27

Witness CSR only requires 5 arguments now.

In exchange for the tight coupling with the global variables, and consequently, reduced reusability and increased difficulty of maintenance.

Elysia · 11-30-2008, 09:51 AM

Alarmingly, I found these errors when checking the code (yes, I combined them inside one file with CSR first).
Warning 2 warning C4101: 'k' : unreferenced local variable g:\w00t\visual studio 2008\projects\temp\temp4.cpp 10
Warning 3 warning C4431: missing type specifier - int assumed. Note: C no longer supports default-int g:\w00t\visual studio 2008\projects\temp\temp4.cpp 48
Warning 4 warning C4101: 'k' : unreferenced local variable g:\w00t\visual studio 2008\projects\temp\temp4.cpp 49
Warning 5 warning C4101: 'j' : unreferenced local variable g:\w00t\visual studio 2008\projects\temp\temp4.cpp 49
Warning 6 warning C6308: 'realloc' might return null pointer: assigning null pointer to 'Ac', which is passed as an argument to 'realloc', will cause the original memory block to be leaked g:\w00t\visual studio 2008\projects\temp\temp4.cpp 19
Warning 7 warning C6308: 'realloc' might return null pointer: assigning null pointer to 'Jc', which is passed as an argument to 'realloc', will cause the original memory block to be leaked g:\w00t\visual studio 2008\projects\temp\temp4.cpp 20
Warning 8 warning C6385: Invalid data: accessing 'A[i]', the readable size is '96' bytes, but '8388488' bytes might be read: Lines: 10, 11, 14, 15, 16, 15, 16, 15, 16, 17 g:\w00t\visual studio 2008\projects\temp\temp4.cpp 17
Warning 9 warning C6011: Dereferencing NULL pointer 'Ic': Lines: 49, 51, 54, 55, 58, 59, 60, 64, 67, 70, 71 g:\w00t\visual studio 2008\projects\temp\temp4.cpp 71
Warning 10 warning C6011: Dereferencing NULL pointer 'Jc': Lines: 49, 51, 54, 55, 58, 59, 60, 64, 67, 68 g:\w00t\visual studio 2008\projects\temp\temp4.cpp 68
Warning 11 warning C6011: Dereferencing NULL pointer 'Ac': Lines: 49, 51, 54, 55, 58, 59, 60, 64, 67, 68 g:\w00t\visual studio 2008\projects\temp\temp4.cpp 68

**laserlight** · 11-30-2008, 09:55 AM

Quote:

Originally Posted by Elysia

Alarmingly, I found these errors when checking the code (yes, I combined them inside one file with CSR first).

They are all warnings, actually. The first one is not necessarily an issue since // is a valid comment marker in C99, and the "'realloc' might return null pointer" warning has already been covered by anon, while I am guessing that the "Dereferencing NULL pointer" are directly related to the current problem that simone.marras brought up.

Elysia · 11-30-2008, 09:59 AM

Quote:

Originally Posted by laserlight

They are all warnings, actually.

Yeah, they are reported as warnings by the compiler, but such things as dereferencing a NULL pointer is an error - perhaps not compile error, but my definition of error is not always just limited to that.

Quote:

The first one is not necessarily an issue since // is a valid comment marker in C99

Indeed. VS only supports C90, and it was in a MS header, so I removed it... You probably saw it before I edited it away.

Quote:

and the "'realloc' might return null pointer" warning has already been covered by anon, while I am guessing that the "Dereferencing NULL pointer" are directly related to the current problem that simone.marras brought up.

Not to rain on anyone's parade, but a little evidence or compiler warnings to back it up doesn't hurt, eh?

Besides, it shows that you should not do this. It also shows that fixing these problems does not raise any flags when using code analysis with VS, which is a good thing, as well!

11-30-2008, 08:05 AM	#3
MK27 subminimalist Join Date: Jul 2008 Location: NYC Posts: 3,946	I don't have "nrutil.h" so I can't try this, but you are passing values (not pointers) to a function that doesn't return anything. If you want to change the values in place, you should pass your function pointers. What do your variables look like before you call CSR? Are they correct? Are they the same inside main as they were before? If so, this is definately the problem. ps. regarding anon's problem about realloc: if realloc returns a NULL pointer everything is pretty much all over anyway, so you can just go: if ((ptr=realloc(ptr,size))==NULL) fatal("realloc fail"); __________________ Accuracy and integrity mean nothing if you don't make it past the censors...PYTHAGORAS Last edited by MK27; 11-30-2008 at 08:08 AM.
MK27 is offline

11-30-2008, 08:09 AM	#4
simone.marras Registered User Join Date: Aug 2006 Posts: 61	Hi Anon, thanks for replying so quickly. I am not sure on what you are suggesting: if I want to fill in a previously empty array Jc of defined of type int *Jc in main(), and passed to CRS as CRS(Jc), why does it not work that way? I mean, why Jc is filled in correctly inside CRS, but when I print it back from main(), I get a different array? From the code that I wrapped, what part is the one that is causing memory leak? Thanks a lot again Simone
simone.marras is offline

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

Similar Threads
Thread	Thread Starter	Forum	Replies	Last Post
temperature sensors	danko	C Programming	22	07-10-2007 07:26 PM
Including lib in a lib	bibiteinfo	C++ Programming	0	02-07-2006 02:28 PM
I have the Memory Address of a function, how do I call it?	vulcan_146	C++ Programming	8	05-22-2005 02:00 AM
Didn't quite know where to post this.......compiler problem...	incognito	C++ Programming	5	02-08-2003 07:42 PM
Manipulating the Windows Clipboard	Johno	Windows Programming	2	10-01-2002 09:37 AM