Help with efficient memory management

[mjoshi]

I have a code which throws "segmentation fault" after some computation.
I was looking for some help with efficient memory management.
My program does the following:
I have a grid with min. i,j,k coordinates -52, -34, -35 and max. i,j,k coordinates 53, 37, 33. (In i, j, k loop).
I want to compare each grid point to a *.pdb file of this sort which has ~2000 lines of x, y, z positions.
ATOM 1756 OH2 TIP3W2710 -46.420 0.150 2.420
ATOM 1757 OH2 TIP3W2753 19.800 2.170 -14.550
ATOM 1758 OH2 TIP3W2754 18.150 20.090 12.270

So basically in the main body of the program i compare each unique i, j, k value to each x, y, z in the compare() function.
The problem is the program goes through 1000 i,j,k values and then gives segmentation fault.
Heres the program:

#include<stdio.h>
#include<stdlib.h>
#include<math.h>
main()
{
FILE *fp1;
float i, j, k;
int val;
 
fp1 = fopen("output.dat", "w");
 
 
 
for (i=-52; i <53; i++)
{
for(j=-34; j <37; j++)
{
for (k=-35; k <33; k++)
{
val= compare (i,j,k);
if (val>=1)
{
fprintf(fp1, "%f\t %f\t %f\t %d\n", i,j,k, val);
}
}
}
}
 
fclose(fp1);
}
compare (p,q,r)
float p,q,r ;
{
 
char cbuff[100];
char sx[7];
char sy[7];
char sz[7];
float a[2500];
float b[2500];
float c[2500];
float dist, num;
int count=0, i=0, j;
 
FILE *fp2;
fp2 = fopen("test1.pdb", "r");
 
while (fgets (cbuff, 80, fp2)!= NULL)
 
{
if (cbuff[0] =='A' && cbuff[1] == 'T' && cbuff[2] == 'O' && cbuff[3] == 'M')
{
strncpy (sx, cbuff+31, 7);
sx[7] = '\0';
strncpy (sy, cbuff+40, 7);
sy[7] = '\0';
strncpy (sz, cbuff+47, 7);
sz[7] = '\0';
a[i] = atof(sx);
b[i] = atof(sy);
c[i] = atof(sz);
/*printf("%d\t%f\t%f\t%f\n", i, a[i], b[i], c[i]);*/
i++;
}
}
 
for(i=0; i < 2013; i++)
{
num = (pow ((c[i]-r),2) + pow ((b[i]-q),2) + pow((a[i]-p),2));
dist = sqrt (num);
if (dist < 1 )
{
count++;
printf("%f\n", dist);
}
}
 
return(count);
fclose(fp2);
}

You're stepping out of memory no doubt. Bad idea to hard code stuff like that.

or (i=-52; i <53; i++)
{
for(j=-34; j <37; j++)
{
for (k=-35; k <33; k++)

[mjoshi]

So whats the alternative ?

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
int main()
{
   FILE *fp1;
   float i, j, k;
   int val;

   fp1 = fopen ( "output.dat", "w" );



   for ( i = -52; i < 53; i++ )
   {
      for ( j = -34; j < 37; j++ )
      {
         for ( k = -35; k < 33; k++ )
         {
            val = compare ( i, j, k );
            if ( val >= 1 )
            {
               fprintf ( fp1, "%f\t %f\t %f\t %d\n", i, j, k, val );
            }
         }
      }
   }

   fclose ( fp1 );
}
compare ( p, q, r )
float p, q, r ;
{

   char cbuff[100];
   char sx[7];
   char sy[7];
   char sz[7];
   float a[2500];
   float b[2500];
   float c[2500];
   float dist, num;
   int count = 0, i = 0, j;

   FILE *fp2;
   fp2 = fopen ( "test1.pdb", "r" );

   while ( fgets ( cbuff, 80, fp2 ) != NULL )

   {
      if ( cbuff[0] == 'A' && cbuff[1] == 'T' && cbuff[2] == 'O' && cbuff[3] == 'M' )
      {
         strncpy ( sx, cbuff + 31, 7 );
         sx[7] = '\0';
         strncpy ( sy, cbuff + 40, 7 );
         sy[7] = '\0';
         strncpy ( sz, cbuff + 47, 7 );
         sz[7] = '\0';
         a[i] = atof ( sx );
         b[i] = atof ( sy );
         c[i] = atof ( sz );
         /*printf("%d\t%f\t%f\t%f\n", i, a[i], b[i], c[i]);*/
         i++;
      }
   }

   for ( i = 0; i < 2013; i++ )
   {
      num = ( pow ( ( c[i] - r ), 2 ) + pow ( ( b[i] - q ), 2 ) + pow ( ( a[i] - p ), 2 ) );
      dist = sqrt ( num );
      if ( dist < 1 )
      {
         count++;
         printf ( "%f\n", dist );
      }
   }

   return ( count );
   fclose ( fp2 );
}

There are many alternatives. To find out where you're going wrong, start putting a load of printf's in between each part. btw I've not changed anything, I've just edited it so others can read it.

[mjoshi]

Can you suggest some alternative to the hard coding of the for loops ? I believe thats whats eating up the memory. The program works fine if i reduce the numbers of i, j and k.

There is quite a lot wrong with your program, the way you parse your data is also very primitive. And if atof is anything like atoi, which I imagine to be, then it's a "let's not go there" type function.

Let me think. Can you explain in plain english what you want it to do?

Can you suggest some alternative to the hard coding of the for loops ? I believe thats whats eating up the memory. The program works fine if i reduce the numbers of i, j and k.

Hmm, if you say it works ok, the math I mean, then there's no point me suggesting changes. Although personally I would have parsed it line by line then used the space as a delimiter to separate into tokens etc.

Anyway the problem is with your memory, as you mentioned.

for ( i = -52; i < 53; i++ )
   {
      for ( j = -34; j < 37; j++ )
      {
         for ( k = -35; k < 33; k++ )
         {

In order for that to work you need to allocate about 100 x 60 x 60 = 360000 on the stack. Is such a size legal.?Probably not. In that case you might need to dynamically increase the array on the fly using malloc.

I don't really use 'c' that often perhaps someone else can help you.

[mjoshi]

I need help with dynamically increasing arrays using malloc.

I have a code which has the following for loops :

for (float i = -52; i < 53; i++ )
   {
      for (float j = -34; j < 37; j++ )
      {
         for (float k = -35; k < 33; k++ )
         {

         (do_some_math);
         }
      }
   }

The problem is that the memory runs out and the program throws a seg fault.

I thought someone else would have helped you since. In the mean time read these?

http://www.cplusplus.com/reference/clibrary/cstdlib/malloc.html

http://vergil.chemistry.gatech.edu/resources/programming/c-tutorial/dynamic.html

[~s.o.s~]

Stop using floating point variables as your for loop counter and you should be good to go.

[mjoshi]

Is it possible to convert integer to float ? I pass the i, j, k from the for loops as floating points to a function where some math is performed.

[~s.o.s~]

The point here is that, are floating point numbers absolutely necessary in your program. Can't your task be performed without floating point numbers? If you still want, you can safely pass integers to functions which accept float and the compiler will take care of the upcasting.

[mjoshi]

Yes, they are for the math. I was able to use typecasting to get float values. Changing the float values to int in the for loops really got my program working. Thanks !

[~s.o.s~]

...and always avoid using floats for loop indices if you want to save yourself from future troubles.

[arld101]

Men, you are calling a *LOT* of times a compare() function that allocates a *LOT* of memory... just declare all those buffers outside the function ONE single time for all, and clean each one after use it.

By the way, you call return and AFTER THAT you close the file. In that order, the file will never be closed, because when you call return, the function ends.