How to properly reallocate a two-dimensional array in C?
Asked Answered
K

3

8

I am trying to load two double numbers from input into a two-dimensional array that is dynamically reallocated by each user input.

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


int main(int argc, char** argv) {

    int count;
    double number1, number2, **numbers;

    while (scanf("%lf,%lf", number1, number2) != EOF) {

        count++;
        numbers = (double**) realloc(numbers, count * 2 * sizeof (double));
        if (numbers == NULL) {
            exit(1);
        }
        numbers[count][0] = number1;
        numbers[count][1] = number2;
    }

    return 0;
}

The program compiles without problems, but it fails every time I try to store a value in the array (it is likely a memory problem).

Can someone show me how to properly reallocate the two-dimensional array in my program?

Kestrel answered 17/11, 2013 at 21:29 Comment(7)
It's a very. A very-very bad approach to reallocate memory every single time when inserting a value.Catarinacatarrh
it is also a very very bad idea to do this with an indeterminate pointer numbers, passed as the first parameter to realloc(). At the very least that thing should be initialized to NULL. And your scanf() should be checking for ==2, not just anything-but-EOF.Muchness
Arrays are not pointers. You are trying to treat a pointer-to-pointer as if it was an array of arrays, which it isn't. Search for a duplicate, there are already dozens of identical questions.Sipple
Furthermore, numbers[count] wouldn't be valid even if numbers was an array of count elements. Array indices start at zero.Sipple
I found this for simple array here https://mcmap.net/q/595353/-resizing-an-array-with-c so i think why it wouldnt work for two dimensionalKestrel
And last but not least, your pointer(s) at numbers[n] is itself indeterminate, so dereferencing it for assignment is also UB. In short, every non-declarative line in this code has UB of one form or another (except return 0;, so you have that going for you).Muchness
@WhozCraig, right you're. This code is a good example of lotta UBs. So 1)Pointers aren't arrays. 2)Indexing begins with 0. 3)Scanf returns the number of items of the argument list successfully filled. 4)Pointers and integers should be initialized with 0 5)Reallocating memory after every input isn't good.Catarinacatarrh
G
25

You have a couple of problems.

  1. You don't initialize numbers = 0; or count = 0 so you have an indeterminate value in the variable before you start the first realloc() call. That's bad news.
  2. The more major problem is that you've misunderstood the memory allocation that's needed to simulate a 2D-array.
  3. Your scanf() call is incorrect; you are not passing pointers to it.

ASCII Art

+---------+
| numbers |
+---------+
     |
     v
+------------+     +---------------+---------------+
| numbers[0] |---->| numbers[0][0] | numbers[0][1] |
+------------+     +---------------+---------------+
| numbers[1] |---->| numbers[1][0] | numbers[1][1] |
+------------+     +---------------+---------------+
| numbers[2] |---->| numbers[2][0] | numbers[2][1] |
+------------+     +---------------+---------------+

You actually need the pointer stored in numbers, the array of pointers, and the array of double. At the moment, you are not allocating the space for the array of pointers, and this is the cause of your troubles. The array of doubles can be contiguous or non-contiguous (that is, each row may be separately allocated, but within a row, the allocation must be contiguous, of course).

Working code:

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

int main(void)
{
    int count = 0;
    double number1, number2;
    double **numbers = 0;

    while (scanf("%lf,%lf", &number1, &number2) != EOF)
    {
        numbers = (double **) realloc(numbers, (count + 1) * sizeof(*numbers));
        if (numbers == NULL)
            exit(1);
        numbers[count] = (double *)malloc(2 * sizeof(double));
        if (numbers[count] == 0)
            exit(1);
        numbers[count][0] = number1;
        numbers[count][1] = number2;
        count++;
    }

    for (int i = 0; i < count; i++)
        printf("(%8.2f, %8.2f)\n", numbers[i][0], numbers[i][1]);

    for (int i = 0; i < count; i++)
        free(numbers[i]);
    free(numbers);

    return 0;
}

NB: This is still not good code. In particular, the increment-by-one-each-time mechanism in use is bad. The meme pointer = realloc(pointer, newsize); is bad too; you can't release the previously allocated memory if the allocation fails. You should use newptr = realloc(pointer, newsize); followed by a memory check before pointer = newptr;.

Input file:

12.34,23.45
34.56,45.67
56.78,67.89
78.90,89.01

Output data:

(   12.34,    23.45)
(   34.56,    45.67)
(   56.78,    67.89)
(   78.90,    89.01)

Not formally run under valgrind, but I'm confident it would be OK.


What is the best solution for saving inputs into array without knowing how many inputs I have to store ? Or maybe it is just this complicated in C compared to Java or PHP?

Except for the 'increment by one' part, this about the way it has to work in C, at least if you want to index into the result using two indexes: numbers[i][0] etc.

An alternative would be to allocate the space as you were doing (except not 'incrementing by one'), and then using an expression to index the array: double *numbers = ...; and numbers[i*2+0] and numbers[i*2+1] in your case, but in the more general case of an array with ncols columns, accessing row i and column j using numbers[i*ncols + j]. You trade the notational convenience of numbers[i][j] against the increased complication of memory allocation. (Note, too, that for this mechanism, the type of the array is double *numbers; instead of double **numbers; as it was in your code.)

The alternatives avoiding 'increment by one' typically use a doubling of the amount of space on each allocation. You can decide to do an initial allocation with malloc() and subsequently use realloc() to increase the space, or you can use just realloc() knowing that if the pointer passed in is NULL, then it will do the equivalent of malloc(). (In fact, realloc() is a complete memory allocation management package in one function; if you call it with size 0, it will free() the memory instead of allocating.) People debate whether (ab)using realloc() like that is a good idea or not. Since it is guaranteed by the C89/C90 and later versions of the C standard, it is safe enough, and it cuts out one function call, so I tend to use just realloc():

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

static void free_numbers(double **array, size_t size)
{
    for (size_t i = 0; i < size; i++)
        free(array[i]);
    free(array);
}

int main(void)
{
    int count = 0;
    double number1, number2;
    double **numbers = 0;
    double maxnum = 0;

    while (scanf("%lf,%lf", &number1, &number2) != EOF)
    {
        if (count == maxnum)
        {
            size_t newnum = (maxnum + 2) * 2;   /* 4, 12, 28, 60, ... */
            double **newptr = (double **)realloc(numbers, newnum * sizeof(*numbers));
            if (newptr == NULL)
            {
                free_numbers(numbers, count);
                exit(1);
            }
            maxnum = newnum;
            numbers = newptr;
        }
        numbers[count] = (double *)malloc(2 * sizeof(double));
        if (numbers[count] == 0)
        {
            free_numbers(numbers, count);
            exit(1);
        }
        numbers[count][0] = number1;
        numbers[count][1] = number2;
        count++;
    }

    for (int i = 0; i < count; i++)
        printf("(%8.2f, %8.2f)\n", numbers[i][0], numbers[i][1]);

    free_numbers(numbers, count);

    return 0;
}

This code was checked with valgrind without problems; all code allocated was freed. Note the use of the function free_numbers() to release the memory in the error paths. That's not critical when it is running in a main() function like here, but is definitely important when the work is done in a function that may be used by many programs.

Greenlaw answered 17/11, 2013 at 22:44 Comment(2)
Thank you very much. I just wanted to ask what is the best solution for saving inputs into array without knowing how many inputs do i have to store ? Or maybe it is just this complicated in C comparing to java or php.Kestrel
size_t newnum = (maxnum + 2) * 2; /* 4, 12, 28, 60, ... */ i dont understand this piece of code. Why it is increasing the memory of array by numbers this big?Broddy
P
1

You're incrementing the count variable too early. The first value it will index into the array will be one, however array indexing starts at zero.

Having the count++ after assigning the new values and initializing count to zero should work. However, read the comments other users have posted, you really want a nicer approach to this problem.

Paternal answered 17/11, 2013 at 22:7 Comment(1)
I was hoping that someone could tell me the nicer approach to this problem. I was programming for some time in java and php and never dealt with memory alocation problem.Kestrel
N
0
#include <stdio.h>
#include <stdlib.h>

void agregar_int(int **,int);
void agregar_char(char **,int); 
char **tz=NULL;
int **tr=0;
int a;

int  main(void){
    a=2;
    for (a=1;a<100;a++)
    {
        agregar_int(tr,a);
    }
    for (a=1;a<100;a++)
    {
        agregar_char(tz,a);
    }
}

agregar_int (int **tr,int a)
{
    printf ("%d----------------------------------------------\n",a);    
    tr = (int**) realloc (tr, (a+1) * sizeof(*tr));
    tr[a] = (int *) malloc (5 * sizeof(int));
    tr[a][0]=a; tr[a][1]=a;tr[a][2]=a;tr[a][3]=a;tr[a][4]=a;
    printf("%d \t %d \t %d \t %d \t %d  \n",tr[a][0],tr[a][1],tr[a][2],tr[a][3],tr[a][4]);
}

agregar_char (char **tz,int a)
{
    printf ("%d----------------------------------------------\n",a);    
    tz = (char**) realloc (tz, (a+1) * sizeof(*tz));
    tz[a] = (char *) malloc (7 * sizeof(char));
    tz[a][0]='E'; tz[a][1]='s';tz[a][2]='t';tz[a][3]='e';tz[a][4]='b',tz[a][5]='a',tz[a][6]='n';
    printf("%c%c%c%c%c%c%c \n",tz[a][0],tz[a][1],tz[a][2],tz[a][3],tz[a][4],tz[a][5],tz[a][6]);
}
Netsuke answered 16/11, 2017 at 15:15 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.