cs_pool_besty
cs_pool_besty

Reputation: 25

Why is this function causing a segfault?

I have a 2D array that I'm trying to pass through to a function, but each time I try to access the array from the function I get a segmentation fault

#include <stdio.h>
#define N 100

void create_magic_square(int n, int magic_square[N][N]);
void print_magic_square(int n, int magic_square[N][N]);


int main(void) {
    int n;

    printf("This program, creates a magic square of a specified size.\nThe size must be an odd number between 1 and 99.\nEnter size of magic square: ");
    scanf("%d", &n);

    int magic_square[n][n];

    create_magic_square(n, magic_square[N][N]);
    print_magic_square(n, magic_square[N][N]);
    return 0;
}

void create_magic_square(int n, int magic_square[N][N]) {

    int i, row, col, next_row, next_col;
    for (row = 0; row < n; row++) {
        for (col = 0; col < n; col++) {
            magic_square[row][col] = 0;
        }
    }
}

The for loop is supposed to fill the array with zeroes and then move on to a while loop, but once it hits 'magic_square[row][col] = 0;', I get a segfault as seen here:

The size must be an odd number between 1 and 99.
Enter size of magic square: 5

Program received signal SIGSEGV, Segmentation fault.
0x0000000000400894 in create_magic_square (n=5, magic_square=0x2e2f706d) at q2.c:26
26                  magic_square[row][col] = 0;

I don't know what to do from here, what might be the cause of the problem?

Upvotes: 1

Views: 75

Answers (1)

Kaz
Kaz

Reputation: 58578

The type signature of

void create_magic_square(int n, int magic_square[N][N]);

is actually:

void create_magic_square(int n, int (*magic_square)[N]);

It takes a pointer to the first element of an array whose elements are arrays of N integers.

This declaration:

int magic_square[n][n];

produces a C99 variable-length ARRAY (VLA): an 5-element array of 5-element arrays of int. This is not compatible with what the function expects. But, never mind that, this is not even passed to the function in any manner that resembles correct:

create_magic_square(n, magic_square[N][N]);

Here, the expression magic_square[N][N] accesses the magic_square out-of-bounds (undefined behavior) to retrieve an int value, which is then passed as a pointer argument. That is a constraint violation; the compiler should produce a diagnostic here (likely to be worded as being about an integer to pointer conversion without a cast).

The expression we want looks like:

create_magic_square(n, magic_square);

but for that to be correct, the function has to receive the VLA in the correct way.

Here is a diff between your program and a working one:

--- magicsquare-ORIG.c  2019-10-10 16:14:50.437827772 -0700
+++ magicsquare.c   2019-10-10 16:17:17.896145951 -0700
@@ -1,8 +1,7 @@
 #include <stdio.h>
-#define N 100

-void create_magic_square(int n, int magic_square[N][N]);
-void print_magic_square(int n, int magic_square[N][N]);
+void create_magic_square(int n, int magic_square[n][n]);
+void print_magic_square(int n, int magic_square[n][n]);


 int main(void) {
@@ -13,17 +12,28 @@

     int magic_square[n][n];

-    create_magic_square(n, magic_square[N][N]);
-    print_magic_square(n, magic_square[N][N]);
+    create_magic_square(n, magic_square);
+    print_magic_square(n, magic_square);
     return 0;
 }

-void create_magic_square(int n, int magic_square[N][N]) {
+void create_magic_square(int n, int magic_square[n][n]) {

-    int i, row, col, next_row, next_col;
+    int row, col;
     for (row = 0; row < n; row++) {
         for (col = 0; col < n; col++) {
             magic_square[row][col] = 0;
         }
     }
 }
+
+void print_magic_square(int n, int magic_square[n][n]) {
+
+    int row, col;
+    for (row = 0; row < n; row++) {
+        for (col = 0; col < n; col++) {
+            printf("%d,", magic_square[row][col]);
+        }
+        putchar('\n');
+    }
+}

We lose the N macro, add a print_magic_square function, call the functions properly, and drop some unused local variables.

Upvotes: 3

Related Questions