Click here to Skip to main content
15,031,962 members
Please Sign up or sign in to vote.
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

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.
   
Comments
pasztorpisti 23-Aug-13 16:52pm
   
It's offtopic but useful like my posts sometimes... +5
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.
   
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.
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.
   
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.
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.
   

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




CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900