memory allocation problem.

Question

0.00/5 (No votes)

See more:

Hi..I have written a c code that calculates difference between corresponding values of two text files. Rows of both text files can vary but columns are fixed. so I used dynamic memory allocation technique. format of text files are like...

5.0 5.0 5.0 5.0 5.0 5.0 5.0 5.0 5.0 5.0 5.0 5.0 5.0
4.0 4.0 4.0 4.0 4.0 4.0 4.0 4.0 4.0 4.0 4.0 4.0 4.0
3.0 3.0 3.0 3.0 3.0 3.0 3.0 3.0 3.0 3.0 3.0 3.0 3.0

The code I have written so far is...

C#

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
void main()
{
    int i=0,j,k,x,count=0,n=0,m=13,p=0,q=13;
    float dis=0.0, s=0.0;
    float *avg;
    float **a;
    float **b;
    char buf[BUFSIZ];
    FILE *fp1,*fp2,*fp3;
    clrscr();
    fp1=fopen("a1.txt","r");
    fp2=fopen("a2.txt","r");
    fp3=fopen("final.txt","wb");
    while (fgets(buf,sizeof(buf),fp1) != NULL)
    n++; // counting rows of a1.txt

    while (fgets(buf,sizeof(buf),fp2) != NULL)
    p++;    // counting rows of a2.txt
    rewind(fp1);
    a = (float**) malloc(n * 13 * sizeof(float *));
    for(i=0;i<n;i++)
    {
        for(j=0;j<m;j++)
        {
            fscanf(fp1,"%f",&a[i][j]);
            printf("%f ",a[i][j]);
        }
       //   printf("\n");
    }
   
    rewind(fp2);
    b = (float**) malloc(p * 13 * sizeof(float *));
    for(i=0;i<p;i++)
    {
        for(j=0;j<q;j++)
        {
            fscanf(fp2,"%f",&b[i][j]);
            printf("%f ",b[i][j]);
        }
          printf("\n\n");
    }
         // printf("\n");
    // rewind(fp1);
       //    rewind(fp2);
    //calculation part
    avg = (float *) malloc(n * sizeof(float));
     for(x=0;x<=(n-1);x++)
     {
        if(count!=(n-p)+1)
        {
            i=x;k=0;
            while(i<(p+x))
            {
                for(j=0;j<q;j++)
                {
                    s=s+pow((a[i][j]-b[k][j]),2.0);
                    dis=sqrt(s);
                }
                i++;
                k++;

            }
            count++;
            avg[i]=dis;
            printf("%f\n",avg[i]);

            s=0;

        }
        else
        {
            break;
        }
        fprintf(fp3,"%f\n",avg[i]);
    }
    getch();
    fclose(fp1);
    fclose(fp2);
    fclose(fp3);
    free(a);
    free(b);
    free(avg);
}

Main problem with this code is that either the final.txt is having all zero values or nothing. I don't know what wrong I am doing. Any help is appreciated. Thank you.

Posted 23-Aug-13 1:12am

Member 10227515

Add a Solution

4 solutions

Solution 2

Some style hints which may help you (if not now, then in the future).

if(count!=(n-p)+1)

Such testing in a conditional, although occasionally correct, is generally dangerous.

Why?
Normally, you are testing for a boundary condition of some sort which you do not want to cross. By writing with an == or !=, you are allowing for all values on either side of the boundary to pass (or fail) the test, with only a single value to satisfy your conditional. Should any item within your loop modify a value in an unexpected way, you could skip over your targeted value: one possible result is that your loop could run on forever. It's thus better to stick with > and < types of tests, blocking out all of creation that is undesirable.

Other items could be a bit more consistently written but, since they'll not cause an error I won't nit-pick them.

one final thought: you noted that you either skip the last row or get a row of all zeros as your last right (should it have non-zero values or is it an extra row?). Presuming your not get this type of error when you use the same code on the same data, you should check your boundaries (< vs <=, for example, and what you are testing). Another diagnostic trick which may be useful for you: allocate an extra element in your malloc's: both in rows and columns. The extra space, if it apparently fixes your problems, indicates that you've probably one block of memory overwriting another: check how you calculate your allocations.

Posted 23-Aug-13 2:52am

W∴ Balboos, GHB

Comments

pasztorpisti 23-Aug-13 16:52pm

It's offtopic but useful like my posts sometimes... +5

Solution 4

Stefan's solution is a possible way to do it. The only disadvantage it has it that it allocates as many memory chunks as there are rows plus on vector for the row pointers. That is probably fast enough for you purposes. But still there is a way to allocate the memory for one file in a single chunk and still address your elements in a two-dimensional array:

C++

float (*a)[13];

a = (float(*)[13]) malloc(n * 13 * sizeof(float *));

/* example */
a[99][12] = 42; // assigns to the 99th row, 12th column

That only works due to the fact that your number of columns is known at compile time.

If both dimensions have to be dynamically sized you can still work with a single malloc. In that case, allocate just a one-dimensional array and calculate the indices by hand. For example the above code could also be written as:

C++

float* a;
int cols, rows;
...
cols = 13;
rows = 100;
a = malloc (rows * cols * sizeof (float));
...
a[99*cols + 12] = 42;

Actually, many people prefer the latter method as it is simple and easy to generalize to more dimensions.

Posted 23-Aug-13 7:48am

nv3

Comments

pasztorpisti 23-Aug-13 14:07pm

5. Would be better with some typedefs! float (*a)[13];!@&#!!!

#define NUM_COLUMNS 13
typedef float TArr[NUM_COLUMNS];

void test()
{
size_t num_rows = 100;
TArr* p = (TArr*)malloc(sizeof(TArr) * num_rows);

int row = 1;
int column = 5;
p[row][column] = 0.5f;

free(p);
}

Stefan_Lang 27-Aug-13 3:22am

In this case it would work becasue you know the number of columns at compile time. I still would prefer a method that isn't dependend on that kind of a priory knowledge to make the allocation function more generally usable.

pasztorpisti 27-Aug-13 8:55am

I don't care which allocation method is in action in OP's code but at least he knows about more methods now with their advantages/disadvantages. In a situation where the size of a dimension is known at compile time the dimension usually remains a compile time constant. Such dimensions often contain items for every member of a given enum in my codebase. The allocation method in a big system is abstracted away anyway. OP puts these allocations into functions like float** allocate_2_dim_array(int rows, int columns); void free_2_dim_array(float** ar); if he wants and then he uses the version of his choice. "Hardcoded" allocation spreading across the codebase isn't good.

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Stefan_Lang · Accepted Answer · 2013-08-23T02:47:00

Solution 1

You're allocating a different structure than you are using. If your file has 10 lines, you allocate a one-dimensional array of 130 elements of type float**. Instead you should allocate an array of 10 elements of type float**, and then for each of these 10 elements allocate an array of 13 floats. You can write a helper function for that:

C++

float** alloc_float_matrix(int rows, int columns) {
   float** matrix = 0;
   if (rows > 0 && columns > 0) {
      matrix = malloc(rows*sizeof(float*));
      for (int row = 0; row < rows; ++rows) {
         matrix[row] = malloc(columns*sizeof(float));
      }
   }
   return matrix;
}

You will also need to relese this row by row. You can write a helper function for that, too.

Posted 23-Aug-13 2:47am

Stefan_Lang

Comments

nv3 23-Aug-13 13:55pm

My 5. As a little addition I have added an alternative.

Stefan_Lang 27-Aug-13 3:34am

Yes, your solution has it's advantages. But mine was only meant to fix the problem (i. e. wrong allocation). Therefore I used code that was easy to understand, not necessarily the most performant or clever.

nv3 27-Aug-13 3:57am

Absolutely, Stefan. That's what I thought.

mbue 26-Aug-13 0:30am

only 3. prevent memory fragmentation, extra calls and special realease functions.


float** alloc2D(const unsigned int x,const unsigned int y)
{
	unsigned char*		buff;
	unsigned int			need,ix;
	float**						ax;
	float*						ay;
	
	ASSERT(0<x);
	ASSERT(0<y);

	need = (x * sizeof(float*)) + ((x*y) * sizeof(float));
	buff = (unsigned char*)malloc(need);
	
	ax = (float**)buff;
	ay = (float*)(ax+x);
	for(ix=0;ix<x;ix++)
	{
		ax[ix] = ay;
		ay += y;
	}
	return ax;
}

Stefan_Lang 27-Aug-13 3:16am

I agree this is better than my solution. But you need to be quite firm in pointer arithmetic or you'll mess up your adresses. As far as I can see your solution is good, but I'm always hesitant to impose too much pointer arithmetic on beginner programmers - they may get carried away with the possibilities and then get difficult to pin down problems at run time.

SaqibRasheed · Accepted Answer · 2013-08-23T02:57:00

Memory allocation issue is in the following chunks:

for(j=0;j<m;j++)>
{
fscanf(fp1,"%f",&a[i][j]);
printf("%f ",a[i][j]);
}

for(j=0;j<q;j++)>
{
fscanf(fp2,"%f",&b[i][j]);
printf("%f ",b[i][j]);
}

you have allocated memory to variables 'a' and 'b' but memory of one of the elements is undefined/unallocated within because of garbage balue is scanned and returned.

you have allocated memory in
a = (float**) malloc(n * 13 * sizeof(float *));

you should add the following line in the loop as
a[i]= (float *) malloc(sizeof(float));

Same for the second chunk of for loop. Similarly, you should also release the memory. I hope it will help you.

memory allocation problem.

4 solutions

Solution 2

Solution 4

Solution 1

Solution 3

Add your solution here

Preview 0

Existing Members

...or Join us