Hi there, I have a very simple code made of a main.c and a function defined in its own .c and .h files.

Now, within the function I need to increase the size of an array according to the loop index in use.
The problem comes when I allocate the array even before useing the realloc. If I use malloc initially to allocate a multiple of 1000 elements, although the total elements that will be actually used to fill in the array are only about 40, I obtain a segmentation fault. The same happens if I use a realloc trhough the loop.

I hope I have explained myself right and to get some help from any of u!
I report the part of my code where this is done (function PSUP.c).

thank you

/******************************************************************
* PSUP(connectivity_file, nnodes)
*
*	Points Surrounding Point (PSUP):
*	
*	Input:
*	- connectivity_file: text file of the connectivity matrix.
*						It must contain the conn matrix in the format:
*						conn(1:nelem, 1:elnnode)
*						where nelem is the mesh number of elements and
*						elnnode is the max number of nnode that any element may have.
*						ex:
*						2 5 3
*						6 3 1
*						3 1 8
*						9 8 5
*						...
*						
*						NOTE that the first column does NOT contain the numbering of the elements!!!
*
*	- nnodet: Number of points in the mesh
*
*	Output:
*	- max_psup is the output: maximum number of nodal contacts inside the mesh
*
*
*	REFERENCES:
*	[1] Lohner,R. (2008), Applied computational fluid dynamics techniques",ed.II, John Wiley and Sons
*
******************************************************************/

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

int PSUP(int CONN[][3], int nnode, int nelem, int max_nnode, unsigned int array_numb)
//int PSUP(int **CONN, int nnode, int nelem, int max_nnode, unsigned int array_numb)
{
	int _ipoin, _inode, _ielem, _ipoi1, _ipoi2, _istor;

	printf("\n #Elements surrounding points#\n");

	//Initialize _esup2:
	int *_esup1;
//	int _esup1[nnode*1000];
	int _esup2[nnode+1];
	int _nesup[nnode+1];
	
	_esup1 = malloc((sizeof(int))*nnode*1000);
	
	/****************************/
	IN THIS PART THERE ARE SOME LINES
	WHERE THE ARRAYS USED NEXT ARE ASSIGNED, AND IT WORKS CORRECTLY.
	iT IS A LONG PART, SO i DONT PUT IT FOR THE SAKE OF READABILITY.
	/****************************/


	for(_ielem=array_numb; _ielem<=nelem-1+array_numb; _ielem++){
		for(_inode=array_numb; _inode<=max_nnode-1+array_numb; _inode++){
		//Update storage counter, storing in _esup1:
			_ipoin = CONN[_ielem][_inode];
			_istor = _esup2[_ipoin] + 1;
						
			_esup2[_ipoin] = _istor;
	//		_esup1 = realloc(_esup1,(_istor+1)*sizeof(int));
			_esup1[_istor] = _ielem;
		
		}
	}
	
free(_esup1);

return; //*_nesup;
}

> int PSUP(int CONN[][3], int nnode, int nelem, int max_nnode, unsigned int array_numb)
> //int PSUP(int **CONN, int nnode, int nelem, int max_nnode, unsigned int array_numb)
OK, so which is it?
Unlike single dimensional arrays, where you can switch the notation from int [] to int* at will, the same is NOT true for higher dimensions.
You can't simply change the declaration without altering what the caller passes to the function.

If you're getting any kind of warnings about calling this function, you really need to fix them, because the code is broken.

Which brings me to the next point.
Even though there is nothing obviously wrong(*) with the malloc/realloc calls, dynamic memory allocation in particular has a problem where the true cause of the problem is often nowhere near where a problem is observed.

Try for yourself, and copy/paste this function into a MINIMAL support program (just a main, and essential declarations), and try to call it. If it crashes, then you know the problem is here. If not, then the problem is somewhere else.

(*)Except for
a) not checking for a NULL result to begin with
b) not allowing for the possibility of realloc returning NULL, thus trashing your pointer to the memory you still have. See previous posts for how/why.

> int PSUP(int CONN[][3], int nnode, int nelem, int max_nnode, unsigned int array_numb)
> //int PSUP(int **CONN, int nnode, int nelem, int max_nnode, unsigned int array_numb)
OK, so which is it?
Unlike single dimensional arrays, where you can switch the notation from int [] to int* at will, the same is NOT true for higher dimensions.
You can't simply change the declaration without altering what the caller passes to the function.

If you're getting any kind of warnings about calling this function, you really need to fix them, because the code is broken.

Which brings me to the next point.
Even though there is nothing obviously wrong(*) with the malloc/realloc calls, dynamic memory allocation in particular has a problem where the true cause of the problem is often nowhere near where a problem is observed.

Try for yourself, and copy/paste this function into a MINIMAL support program (just a main, and essential declarations), and try to call it. If it crashes, then you know the problem is here. If not, then the problem is somewhere else.

(*)Except for
a) not checking for a NULL result to begin with
b) not allowing for the possibility of realloc returning NULL, thus trashing your pointer to the memory you still have. See previous posts for how/why.

1) Hi there Salem, thanks so much for your reply; to first reply about the declariation of the function, the way it is set for the time being (the uncommented version) is the correct one because I am testing this function first inside a main.c where arrays are not pointers. However, the commented version of the function declaration will be used once this function works correctly and I can port it into my real code elsewhere. So, no worry about the declaration for the time being.

2) During compilation I have in fact no warnings (or errors) of any sort, and if furthermore, instead of using malloc to allocate *_esup1, I declare _esup1 as a local array with a fixed size of 1000 elements, the code executes correctly without any fault.

That is what I dont really explain: why I cannot use a malloc declaration inside this function.

Thank you again
Best
S

> for(_inode=array_numb; _inode<=max_nnode-1+array_numb; _inode++)
So is this always 0 to 3 ?
To match your CONN array.

Checking the array indices would be a start, say
assert( i >= 0 && i < 3 );

If you compile a debug build, and run the code in the debugger, you should automatically break at the first assert which fails.
You can then examine the variables and your logic.

> That is what I dont really explain: why I cannot use a malloc declaration inside this function.
In a correct program, there would be no problem.

Despite the fact that your static array 'works', I would still suspect that you're running off the ends in some way. It's just that with a static array you get away with it.

Hi again, thanks for replying; I have never used assert(), how would you use it inside this type of code? would that be inside a loop in the CONN array columns?

thanks again
All best
s.

> for(_inode=array_numb; _inode<=max_nnode-1+array_numb; _inode++)
So is this always 0 to 3 ?
To match your CONN array.

Checking the array indices would be a start, say
assert( i >= 0 && i < 3 );

If you compile a debug build, and run the code in the debugger, you should automatically break at the first assert which fails.
You can then examine the variables and your logic.

> That is what I dont really explain: why I cannot use a malloc declaration inside this function.
In a correct program, there would be no problem.

Despite the fact that your static array 'works', I would still suspect that you're running off the ends in some way. It's just that with a static array you get away with it.

How is like I showed you.

Where is anywhere you like where you wish to assert that something "must" be true at this point in the code.

Hi Salem, thanks a lot for all the help; I'll figure it out and get the problem solved one way or another

All the best and merry xmas!
s

You can use the reference if you aren't sure of how the assert function works. I think in your program, you've to "assert" that the value of the variable _inode stays within 0 to 3 and this has to be done in the for loop. If the value of that variable is anything other than 0-3, then an error is logged and the program is terminated instead of giving you a segmentation fault.

This article has been dead for over six months. Start a new discussion instead.