Dynamic allocation of 2 dimensional array in c /linux

Question

I just can't figure out how to do a malloc. The following code just types the first 5 lines and then stops, any help would be appreciated!

// Read query points from query file------------------------------
double **queryPoint;

token=(char*)malloc(40);
int qp_count=0;

i=0;
qp_count=0;
while(fgets(line,sizeof(line),queryFile)!=NULL)
{
    queryPoint=(double*)malloc(sizeof(double**));
    queryPoint[qp_count]=(double*)malloc(sizeof(double*)*2);

    printf("line:%s",line); 

    token = strtok(line," ,\t");        

    queryPoint[qp_count][0]=atof(token);        
    token = strtok(NULL, " ,\t");

    printf("l[%d]=%lf \n",qp_count,queryPoint[qp_count][0]);

    queryPoint[qp_count][1]=atof(token);
    token = strtok(NULL, " ,\t");
    printf("l[%d]=%lf \n",qp_count,queryPoint[qp_count][1]);

    qp_count++;
}

{ This is the form of the query file

9.85797 5.72533
9.58711 2.09899
2.28203 7.19344
4.49096 5.50094
6.05297 1.60751
6.19901 1.52312

} ... 30 lines total

Answer 1

Initialize at the beginning:

double **queryPoint = 0;
int qp_count = 0;

For every line call:

// call realloc to make the space for points larger by one element
queryPoint = realloc(queryPoint, sizeof(double*)*(qp_count+1));

// allocate space for the new point
queryPoint[qp_count] = malloc(sizeof(double)*2);

// increase the point count
qp_count++;

Note that you may use realloc to resize the space for lines. Also note that you should add error handling.

To free the memory you need to call:

for(int i=0;i<qp_count;i++)
    free(queryPoint[i]);

free(queryPoint);

Answer 2

queryPoint=(double*)malloc(sizeof(double**));
queryPoint[qp_count]=(double*)malloc(sizeof(double*)*2);

You've clearly got yourself confused with casting, sizes and various other properties of malloc so let's start with the syntax of a proper malloc.

Firstly, in C it is unnecessary to cast the result of the malloc operation as it always returns a pointer - the type of a pointer refers to the type of the pointed data, rather than the pointer itself, which will always be the size of a register. In C++, however, it is mandated to cast it.

Now, the general syntax of a malloc is this:

TYPE* pointer = malloc(n * sizeof(TYPE));

Where TYPE is some type. The sizeof should always be one level of indirection less than the pointer to which you're allocating. (So TYPE** give you a malloc of TYPE*). n is the number of blocks of this size to allocate, so if you want to allocate 100 doubles, that's what n is.

In any case, you have declared double** queryPoint; so your first malloc should be:

queryPoint = (double**) malloc(some_size*sizeof(double*));

This gives us an array of pointers of size some_size. Just like any other array, you can realloc this as you need to, although pre-determining the amount is probably ideal. Then, for each line you wish to allocate, you simply pick an offset from queryPoint and allocate an array of doubles to which that particular pointer points, like so:

queryPoint[i] = (double*) malloc(sizeof_this_array*sizeof(double));

Then, access to a specific point in the 2D array is via two subscripts: queryPoint[x][y];

As others have said, realloc is an option, but I advise every time you overfill the array you add a fixed amount on, or simply double what you have, as memory is (relatively) cheap and this will save a system call or six.

Now, I've talked about pointers etc so I'm going to draw an obligatory memory table so you can see what this looks like:

|Address      | Contents      | Comments
|-------------|---------------|-------------------------
|0x12345      | 0x20000       | double** queryPointer
| ...         | ...           | ...
|0x20000      | 0x30000       | double* queryPointer[0]
|0x20001      | 0x30016       | double* queryPointer[1]
|0x20002      | 0x30032       | double* queryPointer[2]
| ...         | ...           | ...
|0x30000      | 0183737722122 | These together make up the floating 
|0x30001      | 0183737722122 | point at double queryPointer[0][0]
|0x30002      | 0183737722122 |
| ...         | ...           | ...
|0x30016      | 0183737722122 | These together make up the floating 
|0x30017      | 0183737722122 | point at double queryPointer[0][1]
|0x30018      | 0183737722122 |
| ...         | ...           | ...

A pointer is just an address containing an address, so 0x12345 simply points to the start of a set of addresses from the first malloc. This is an array of pointers, so just a collection of memory addresses containing memory addresses, which point to actual values, as depicted by the 0x3*** range. Note that all address sizes, data sizes and value representations are pretty much garbage.

That is what is going on.

Also, don't forget to free() for each memory address you allocate.

Answer 3

You leak the space allocated by the token = malloc(40); line; that's not good.

You don't show the definition of line.

Each time round the loop, you leak the space previously allocated to queryPoint.

Your casting on queryPoint=(double*)malloc(sizeof(double**)); is wrong; double **queryPoint; should not be assigned a double *. Opinion is split (not necessarily evenly) on whether the cast is a good idea at all.

Each time around the loop after the first, you access space out of the area allocated to queryPoint.

You initialize int qp_count = 0; and then set it to zero again with qp_count = 0; where one of the two assignments is sufficient.

Your second strtok() in the loop could be looking for newline too.

You don't check that strtok() found a token, either time. You don't check that atof() was successful. (Actually, it is not possible to check atof(); you would probably need to use strtod() instead.)

Answer 4

You are on the right track. queryPoint is basically an array of arrays. However, each time through the loop, you replace the previous queryPoint pointer with a new one to a single double*. What you actually want to do is grow queryPoint by one pointer:

queryPoint = (double**)realloc(queryPoint, sizeof(double*)*(qp_count+1));

(Note that for this to work queryPoint should be initialized to NULL.)

Also, your second allocation should be sizeof(double)*2, not sizeof(double*)*2.

Notice the pattern: inside the malloc (or realloc) we have a sizeof(TYPE). The return value is then cast to TYPE*. For example:

p = (double*)malloc(sizeof(double)*n);
pp = (double**)malloc(sizeof(double*)*n);

Answer 5

How many bytes do you expect to be allocated on this call?

queryPoint=(double*)malloc(sizeof(double**));

How many did malloc( ) actually give you?

It gave you however many bytes there are in a void *, probably eight on your 32-bit machine.

What's going on here?

queryPoint[qp_count]=(double*)malloc(sizeof(double*)*2);

You're storing right back into queryPoint, overlaying the pointer that malloc( ) gave you in the immediately previous line. Is that what you intend?

Answer 6

You should first define more precisely what you are trying to do.

You don't know how many lines are in the file, so you don't know initially how many lines the two dimensional array must have. The initial problem/question seams bogus. A two dimensional array is not appropriate for such type of conditions.

A linked list seems more appropriate. If there are only two values per line (row) a structure might be more appropriate so that you can add the pointer to the next queryPoint pair.

Answer 7

This is how you malloc a two-dimensional array:

double (*two_dim_array)[ncols] = malloc(nrows * sizeof *two_dim_array);

Answer 8

The variable queryPoint is each time allocated room for one pointer, which is available at index 0 (queryPoint[0]). You try to adress memory at queryPoint[qp_count]. There is no memory at this index. To fix, allocate queryPoint with as much pointers as you need to read your file.

int MAX_POINTER_COUNT = 10000 // whatever you choose
double** queryPoint=(double**)malloc(MAX_POINTER_COUNT * sizeof(double**));

Be sure not to write behind the end of the array! Doing so is a potentially exploitabele buffer overflow error. (especially if you read data supplied from the outside).