-2

I've got stuck on compiling two-dimensional array in C using malloc();

Trying to compile my code in Microsoft Visual Studio Express 2013 I keep getting error 'a value of type "int **" cannot be assigned to an entity of type "int *"'

Here is my code:

  int main() {
    int lines = 0;
    int **cord;

    FILE *f = fopen("data.txt", "r");
    if (f == NULL) return 0;

    fscanf(f, "%d", &lines);
    printf("%d", lines);

    *cord = (int**)malloc(lines*sizeof(int*));

    int i = 0;
    for (i; i < lines; ++i)
    {
        cord[i] = (int*)malloc(2 * sizeof(int));
        fscanf(f, "%d", cord[i][0]);
        fscanf(f, "%d", cord[i][1]);
    }
    i = 0;
    return 0;
}

Could anyone point me what is wrong there?

happyN
  • 3
  • 1
  • 2
    In C don't cast the result of `malloc` but *always test* against failure of `malloc`; also enable all warnings & debug info. – Basile Starynkevitch Oct 29 '14 at 10:08
  • This is not a 2D array but only an emulation of it. You can define and initialize your array with a single call to `malloc` as `int (*cord)[2] = malloc(2*lines*sizeof(int));`. – Jens Gustedt Oct 29 '14 at 13:17

4 Answers4

2

First, DO NOT cast the return value of malloc().

Regarding the error, in your code, first you need to allocate memory to cord, then to cord[i] (or *cord).

Instead of using

*cord = malloc(lines*sizeof(int*));,

use

cord = malloc(lines*sizeof(int*));

Also, add a NULL check for success of malloc(), like

if (!cord)   //also for cord[i]
{
    //print error and return / exit;
}
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
2
*cord = (int**)malloc(lines*sizeof(int*));

is wrong, you are dereferencing an uninitialized pointer cord. Please code

cord = malloc(lines*sizeof(int*));
if (!cord) { perror("malloc cord"); exit (EXIT_FAILURE); }

Never forget to test against failure of malloc (you should also test it when assigning cord[i])

BTW, you should enable all warnings and debug info in your compiler. If using a recent GCC as gcc -Wall -Wextra -g you would have been warned for the first mistake.

Also, if you want a matrix in C, better use a one-dimensional array to represent it.

Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547
1
*cord = (int**)malloc(lines*sizeof(int*));

The variable cord has type int **, so the expression *cord has type int *. You're casting the result of malloc to int **. Thus, you're attempting to assign a value of type int ** to an expression of type int *, which is an error. That line should be written as

cord = malloc( lines * sizeof *cord ); // note no cast, operand of sizeof

Style note - whitespace is your friend. Use it. Your eyes aren't going to be 20 years old forever.

If the number of columns is known at compile time (as it appears to be in this case), you can do a one-step allocation:

int (*cord)[2];
...
cord = malloc( lines * sizeof *cord ); 
if ( cord )
{
  for ( int i = 0; i < lines; i++ )
  {
    fscanf( f, "%d", &cord[i][0] );
    fscanf( f, "%d", &cord[i][1] );
  }
  ...
  free( cord );
}
else
  // handle allocation failure

The variablecord has type "pointer to 2-element array of int", so the expression *cord has type "2-element array of int". sizeof *cord gives us the number of bytes required for each subarray, so lines * sizeof *cord gives us the total number of bytes required for the whole 2D array.

Advantages - single-step allocation (meaning single-step deallocation), and all the rows in the array are contiguous.

Disadvantages - May fail for very large arrays if memory is sufficiently fragmented, requires that you know the number of columns at compile time or that your compiler supports variable-length arrays (which were introduced in C99 and are optional as of C2011).

In general, your malloc statements should be written as

T *ptr = malloc( N * sizeof *ptr ); // calloc( N, sizeof *ptr )

or

T *ptr = NULL;
...
ptr = malloc( N * sizeof *ptr );  // calloc( N, sizeof *ptr );

C doesn't require you to cast the result of malloc/calloc1. C++ does require it since it doesn't allow implicit conversion of void pointers, but if you're writing C++ you should be using new and delete instead of malloc and free.


1. As of the 1989 standard, anyway; prior to C89, malloc and calloc returned char * instead of void *, so for pre-C89 compilers the cast was required. MSVC supports the C89 standard, so you don't need the cast
John Bode
  • 119,563
  • 19
  • 122
  • 198
0

Change

*cord = (int**)malloc(lines*sizeof(int*));

to

cord = malloc(lines*sizeof(int*));

Also, you are not allocating a 2D array, but rather a pointer-based lookup table. To allocate a real 2D array, do like this.

Community
  • 1
  • 1
Lundin
  • 195,001
  • 40
  • 254
  • 396