0

I have searched but could not find the appropriate solution. The code generates a 2D array in a function by taking size of the array as the arguments. Now in main() function, I want the array to be printed. But the output is not correct. This is what I have got so far.

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

int * generate(int size){
    int i, j;
    // allocate  memory
    static int **X = (int **)malloc(size * sizeof(int *)); 
    for(i=0; i<size;i++)
        X[i]=(int *)malloc(size * sizeof(int));
    
    for(i=0; i<size;i++)
        for(j=0;j<size;j++)
            X[i][j]=rand()%1000;
    
    return *X;
}

int main(){

    srand(time(NULL));
    int size=3;
    int *p=generate(size);
    
    for(int i=0; i<size; i++){
        for(int j=0; j<size;j++){
            printf("%5d ",(*(p+i)+j));
        }
        printf("\n");
    }
    return 0;
}
user3804152
  • 115
  • 2
  • 8
  • 6
    You *don't* have a 2D array. You have a 1D array of pointers to other 1D arrays, which are not contiguous. – Weather Vane Oct 13 '20 at 08:26
  • Works for me if I remove the `static`: https://ideone.com/P7qU4o – Eugene Ryabtsev Oct 13 '20 at 08:31
  • So what changes can be done here so that if I get the location A[0][0], the elements can be printed using *(*(p+i)+j) – user3804152 Oct 13 '20 at 08:31
  • Please explain _"output is not correct"_. What exactly is not correct? Show the actual output, and show the expected output. – paddy Oct 13 '20 at 08:32
  • For example sample output: [20 21 22] [10 11 12] [30 31 32] So values in next column are just being incremented by 1 – user3804152 Oct 13 '20 at 08:35
  • The problem is if I don't declare dynamic 2D array, it will not be available when the scope of the function is over. – user3804152 Oct 13 '20 at 08:40
  • Well, you told it to. `*(p+i)+j` is the same as `p[i] +j`. Perhaps you wanted `p[i * size + j]`... That would work if you did the _normal_ kind of contiguous 2D allocation, but you've actually done something crazy which is to allocate each row individually and then only return the first row from your `generate` function. So it's impossible for you to access any row except the first. You don't know where it will be in memory. – paddy Oct 13 '20 at 08:40
  • Then what @WeatherVane says. In `generate()` you make three one-dimensional arrays and return the first of them. Nothing can be done in `main()` until that part is fixed. – Eugene Ryabtsev Oct 13 '20 at 08:42
  • A simple route is to make a contiguous 1D array with `int *X = malloc(size * size * sizeof *X)` and then in `main()` you can access it with `X[i * size + j];` Since the aray is dynanic, it doesn't matter where you created it. – Weather Vane Oct 13 '20 at 08:46
  • Thanks to all for the response! The problem lies behind the non-contiguous memory allocation. – user3804152 Oct 13 '20 at 09:12
  • See [Correctly allocating multi-dimensional arrays](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays) which shows how to allocate a 2D array and ends with an example answering this question. – Lundin Oct 13 '20 at 11:39

2 Answers2

2

This is more C as C++, but:

Use malloc(size*size*sizeof(int)) within generate or change generate to return int** instead of int*. (The generate in your case returns *X, and you're getting only the first row).

With the first approach you're getting continuous memory space, with the second not(array of arrays).

StPiere
  • 4,113
  • 15
  • 24
0

You might be after this?

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

int** generate(int size) {
    int i, j;
 
    int ** X = (int **) malloc(sizeof(int*) * size );

    for (i = 0; i < size; i++)
       X[i] = (int*) malloc(sizeof(int) * size);

    for (i = 0; i < size; i++)
    {
        for (j = 0; j < size; j++)
        {
            X[i][j] = rand() % 1000;
            printf("%5d", X[i][j]);
        }
        printf("\n");

    }
    return X;
}

int main() {

    srand(time(NULL));
    int size = 3;
    int** p = generate(size);

    for (int i = 0; i < size; i++) {
        for (int j = 0; j < size; j++) {
            printf("%5d ", p[i][j]);
        }
        printf("\n");
    }

    //DONT FORGET TO FREE
    for (int i = 0; i < size; i++)
    {
      int* currentp = p[i];
      free(currentp);
     }
    return 0;
}

If you want to go though the elements with j + i I think you need to rethink how you are reading the indices. If you read **(p+i) for example, you will get the first element of each row, but I am not sure you know how memory is being allocated for the rest (which I am not sure makes it easy to go looking for the elements as j+i). If you wanted to do that I suspect you need to allocate the array alltogether as size*size so you ensure continuity of indices...

(The code above and the comment that follows are basically the points that StPiere made )

Also, as a suggestion check this post (on why it is not very good to use malloc()/free() in C++)

Silvia
  • 348
  • 2
  • 9