2

I am trying to code a program in C that generates a spiral based on user input and prints it to the console. I cannot figure out how to access the 2D array "data" that I defined in the struct "Spiral". How do I fix the "warning: assignment from incompatible pointer type" error?

#include <stdio.h>

typedef struct Spiral {
    int size;
    int **data;
} Spiral;

Spiral generateSpiral(int size);
void printSpiral(Spiral spiral);
static int rotate();

int main() {
    int size;
    scanf("%d", &size);
    Spiral spiral = generateSpiral(size);
    printSpiral(spiral);
    return 0;
}

Spiral generateSpiral(int size) {
    int data[size][size];
    int i;
    for (i = 0; i < size; i++) {
        int j;
        for (j = 0; j < size; j++) {
            data[i][j] = 0;
        }
    }

    for (i = 0; i < size; i++) {
        data[0][i] = 1;
    }

    int currX = 0;
    int currY = size - 1;

    for (i = size - 1; i > 0; i -= 2) {
        int j;
        for (j = 0; j < 2; j++) {
            int k;
            switch (rotate()) {
                case 0:
                    for (k = 0; k < i; k++) {
                        data[++currX][currY] = 1;
                    }
                    break;
                case 1:
                    for (k = i; k > 0; k--) {
                        data[currX][--currY] = 1;
                    }
                    break;
                case 2:
                    for (k = i; k > 0; k--) {
                        data[--currX][currY] = 1;
                    }
                    break;
                case 3:
                    for (k = 0; k < i; k++) {
                        data[currX][++currY] = 1;
                    }
                    break;
            }
        }
    }
    Spiral spiral;
    spiral.size = size;
    spiral.data = data;
    return spiral;
}

void printSpiral(Spiral spiral) {
    int i;
    for (i = 0; i < spiral.size; i++) {
        int j;
        for (j = 0; j < spiral.size; j++) {
            switch (spiral.data[i][j]) {
                case 0:
                    printf(" ");
                    break;
                case 1:
                    printf("#");
                    break;
            }
        }
        printf("\n");
    }
}

static int rotate() {
    static int val = 0;
    int tmp = val;
    val++;
    if (val > 3)
        val = 0;
    return tmp;
}
Daniel
  • 29
  • 1
  • 1
  • 3
  • 3
    type of `data`(int data[size][size];) is not `int **`. also it can not be used outside a function pointer to an automatic local variables. – BLUEPIXY Oct 19 '14 at 18:45
  • Wait, this compiles `int data[size][size];` when `size` is not const? Anyways `int data[][]` is a different type than `int** data` – luk32 Oct 19 '14 at 18:46
  • 1
    @luk32 `int data[size][size]` is a normal [variable-length array](http://en.wikipedia.org/wiki/Variable-length_array). – Some programmer dude Oct 19 '14 at 18:48
  • 2
    If `spiral.data = data;` didn't flag a compiler warning, jam up your warning level, because its wrong. They're not the same types, not that it matters, as you're saving a local automatic address then expecting it to be valid caller-side after the function returns and the `data` array is no longer valid. – WhozCraig Oct 19 '14 at 18:48
  • @WhozCraig I think that is precisely the line that gives the warning/error, as there is clearly a type mismatch there. I think if it was solved, next thing would be assigning a local variable that goes out of scope. It probably stopped at the 1st thing that was wrong. – luk32 Oct 19 '14 at 18:54
  • @luk32 I hope it is indeed. The OP didn't specify which line was the specific warning. – WhozCraig Oct 19 '14 at 18:55
  • @WhozCraig You are right, that is the line that gives the warning. Sorry for not including that in the original post. – Daniel Oct 19 '14 at 20:51

2 Answers2

7

In the generateSpiral function you make the structures pointer point to the local variable data, but when the function returns data goes out of scope so the pointer now points to unallocated memory leading to undefined behavior.

But that's not your only problem: A second problem is that a pointer to a pointer is not the same as an array of arrays, the memory layout is different.


For the last part, lets check an example. Lets say we have the following declaration

int a[2][2];

In memory it will look something like this:

+---------+---------+---------+---------+
| a[0][0] | a[0][1] | a[1][0] | a[1][1] |
+---------+---------+---------+---------+

In other words, all data is contiguous.

If you, on the other hand have a declaration like

int **p;

and allocate data for it correctly, it will look something like

+------+------+-----+
| p[0] | p[1] | ... |
+------+------+-----+
  |      |      |
  |      |      v
  |      |      ...
  |      v
  |      +---------+---------+-----+
  |      | p[1][0] | p[1][1] | ... |
  |      +---------+---------+-----+
  v
  +---------+---------+-----+
  | p[0][0] | p[0][1] | ... |
  +---------+---------+-----+

The memory is no longer contiguous. There is no longer any maximum size, a pointer points to a contiguous area of memory, but there is no way of knowing how big that area is. You have to keep track of it yourself.


The simple solution to both the problems is to use only pointer to pointer, and then allocate dynamically of the heap:

int **data;

// First allocate memory for `size` number of pointers
// I.e. we allocate an "array" of pointers
data = malloc(size * sizeof(int *));

// Then allocate each entry in the above allocated "array"
// I.e. make each pointer in the "array" point to an "array" of `int`
for (int i = 0; i < size; ++i)
    data[i] = malloc(size * sizeof(int));

Now the local variable data can be used directly to assign to spiral.data.

But there is a catch: In Java you don't have to free memory you allocate, it's handled automatically. In C it's not handled automatically, you have to manually free the memory you allocate or you will have a memory leak.

Freeing the memory can be done like

// First free all the "sub-arrays"
for (int i = 0; i < size; ++i)
    free(spiral.data[i]);

// Then free the top-level "array"
free(spiral.data);

Regarding pointers, a pointer can point to any memory address, and there is really no safety or checking that it points to a valid location. Also, C does not do deep copying of values and structures, if you have a pointer and make it point somewhere, then the compiler or runtime system doesn't make a copy.

And about scoping, Java has local variables as well, and just like in C when a function returns those go out of scope. The difference between Java and C, is that if you return a reference to a local object, then Java keeps track of that and keep the object in memory as long as there are references to the object. C doesn't have references, a pointer is just an integer whose value is an address in memory, and the data pointed to have no idea that there are pointers to it or how many.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • How do I fix it? I'm coming from Java, trying to learn C, and don't entirely understand scope and pointers in C. – Daniel Oct 19 '14 at 20:55
  • Generally, you want to declare the instance of `Spiral` in main and pass a pointer to `generateSpiral`, e.g. `Spiral generateSpiral(Spiral *sp, int size)`, you can then operate on it directly or declare the function as `Spiral *generateSpiral` and return a pointer. – David C. Rankin Oct 20 '14 at 00:08
  • 1
    @Daniel Updated my answer with the reason an array of arrays is not the same as a pointer to a pointer. Also added a possible solution to both problems. – Some programmer dude Oct 20 '14 at 07:27
  • @Someprogrammerdude dude I am not sure you ensured contigious allocation with the implementation you provided... – codemonkey Oct 30 '20 at 16:33
  • @codemonkey No it's not, it's a *jagged array*, and not contiguous. – Some programmer dude Oct 30 '20 at 16:40
0

Your problem is due to a mismatch between the int and the unsigned int returned by the vectors size function. Change your int i , j to type size_t so they are also an unsigned integer like the size of the vectors. This is why you are getting the warning

Ryan Mckenna
  • 104
  • 6