1

I have written the following code to dynamically allocate a two-dimensional array, loop through it to take user inputs, store them and finally return it.

int **input(const int row, const int column, const char *message)
{
  printf("\n%s \n", *message);

  int **matrix = (int **)calloc(row, sizeof(int *));

  for (int a = 0; a < column; a++)
    *(matrix + a) = (int *)calloc(column, sizeof(int));

  /* scanf */
  for (int r = 0; r < row; r++)
  {
    for (int c = 0; c < column; c++)
    {
      printf("E[%d][%d]: ", r + 1, c + 1);
      scanf("%d", ((matrix + r) + c));
    }
  }
  /* endscanf */

  return matrix;
}

When I invoke this function, the program exits without any notice or warning. It also doesn't show any log.

int **test = input(2, 2, "Test:");

What am I doing wrong here?

msrumon
  • 1,250
  • 1
  • 10
  • 27
  • 1
    In the printf you should remove the `*` before message. – Eraklon Feb 28 '20 at 08:52
  • 2
    First of all, please stop using pointer arithmetic for simple "array" indexing. `*(matrix + a)` is *exactly* equal to `matrix[a]`. The latter is usually easier to read and understand (and therefore to maintain), especially if you want to nest indexing. Also in C you don't have to (and really shouldn't) [cast the result of `malloc`, `calloc` or `realloc`](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – Some programmer dude Feb 28 '20 at 08:52
  • This question could be considered a duplicate of this one: [**Correctly allocating multi-dimensional arrays**](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays) – Andrew Henle Feb 28 '20 at 12:42
  • Detail: `int (*matrix)[row][column] = malloc(sizeof *matrix);` allocates for a 2D array. OP's code allocates for a 1D array (of `int *`) and then multiple 1D array of `int`. – chux - Reinstate Monica Feb 28 '20 at 13:55
  • @AndrewHenle, no. I would argue this is a duplicate of that question. I am trying to return the created array from within the function. The question has been discussed about allocating 2D array, not about how to return. Because he didn't mention about returning array. – msrumon Feb 28 '20 at 15:27

2 Answers2

0

Because of the *(p + i) equivalence to p[i] (mentioned in my comment), it follows that *p is equal to p[0].

That means in your first printf call when you dereference message (as *message) what you are passing as argument is really message[0], which is a single character and not a pointer to a null-terminated string as expected by the %s format. That leads to undefined behavior and probable crash.

A good compiler should be able to detect this and warn about it. If it doesn't then you need to enable more verbose warnings.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • Thank you for enlightening me. Hence, I marked your answer as 'correct' one. – msrumon Feb 28 '20 at 10:33
  • Can you help me fixing this issue please? The detail is here: https://github.com/msrumon/mini-c-programs/issues/1 – msrumon Feb 28 '20 at 16:23
  • @msrumon Please read the error message carefully, it tells you exactly what type you're really passing to the function and what you should have for the argument (and no, [an array of arrays is *not* the same as a pointer to a pointer](https://stackoverflow.com/questions/18440205/casting-void-to-2d-array-of-int-c/18440456#18440456)). – Some programmer dude Feb 28 '20 at 18:49
  • I have read the error message, but I didn't understand it fully. That's why I did some experiments. And the results are pretty much identical. Can't I just iterate over the pointers I created dynamically with `calloc()` function and store values into which they point to? – msrumon Feb 29 '20 at 07:03
  • @msrumon An array decays to a pointer to its first element, but that decay isn't "chained". An array of arrays will decay to a pointer to an array. Your function expects an argument that have the type `int **`, but the actual argument you pass is of the type `int (*)[3]`. So if you change your type to match what is actually passed it should work: `int (*matrix)[3]`. – Some programmer dude Feb 29 '20 at 10:38
  • @msrumon Alternatively you could rearrange the order of your arguments for the `splt` function, and use *variable-length arrays*, like `void spit(cconst unsigned rows, const unsigned columns, int (*matrix)[columns], const char *message)` Then you could pass a "matrix" of any size to the function. – Some programmer dude Feb 29 '20 at 10:40
  • Would you mind giving it a try? [Here](https://github.com/msrumon/mini-c-programs/blob/4a5ba1c9c5df133791a59685b41fae1851509181/lib/matrix.h) is the header file. I have also seen that [you can't use pointer arithmetic with dynamically-created pointers](https://stackoverflow.com/a/42094467/11317272). Need to wrap my head around this also. – msrumon Feb 29 '20 at 10:48
0

Here your program corrected:

int **input(const int row, const int column, const char *message)
{
    printf("\n%s \n", message);

    int **matrix = (int **)calloc(row, sizeof(int *));

    for (int a = 0; a < column; a++)
        matrix [a] = (int *) calloc(column, sizeof(int));

    /* scanf */
    for (int r = 0; r < row; r++)
    {
        for (int c = 0; c < column; c++)
        {
            printf("E[%d][%d]: ", r , c );
            scanf("%d", (&matrix [r][c]));
        }
    }
    /* endscanf */

    return matrix;
}

int main ()
{
    int row =2;
    int column = 2;

    int **test = input(row, column, "Test:");

    for (int r = 0; r < row; r++)
    {
        for (int c = 0; c < column; c++)
        {
            printf("E[%d][%d]: %d\n", r , c, test[r][c]  );
        }
    }

    // Free memory 
    for (int r = 0; r < row; r++)
    {
        free (test[r]);
    }

    free (test);

}
Landstalker
  • 1,368
  • 8
  • 9
  • *What* changes did you make to "correct" the problem? The only important modification (that I can see) is very small and easy to miss. And *why* did you make that change? Lastly please take some time to read (or refresh) on [how to write good answers](https://stackoverflow.com/help/how-to-answer). – Some programmer dude Feb 28 '20 at 09:14
  • @Someprogrammerdude `Matrix form [] []` which is more preferable in general because it is easier to interpret especially when our program risks being reiterated by another person. freeing up memory because this is a very important aspect that should not be overlooked. That was all to help him ... – Landstalker Feb 28 '20 at 09:26