Segmentation fault on trying to pass a recursive function to populate a 2D array

Question

I'm trying to pass a recursive function that populates my 2D array of structs. My memory allocation is working fine, but when I try to do a recursion, I get the error: Segmentation fault (core dumped). Any idea why this must be happening? I think I wrote my code so that no index out of bound occurs. I still don't know why this is happening. Any help is going to be appreciated. Thanks!

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

typedef struct {
    char val;
    bool filled;
} elements;

void assign(elements ** elements, int row, int column, int x, int y, int limit);

int main(int argc, char** argv)
{
    int row = 0;
    int column = 0;
    int x = 0;
    int y = 0; 
    int limit = 0;

    sscanf(argv[1], "%d", &row);
    sscanf(argv[2], "%d", &column);
    sscanf(argv[3], "%d", &x);
    sscanf(argv[4], "%d", &y);
    sscanf(argv[5], "%d", &limit);

    elements **foo;

    foo = (elements **)malloc(sizeof(elements *) * row);
    for (int i = 0; i < column; i++)
        foo[i] = (elements *)malloc( sizeof(elements) * row);

    foo[y][x].val = 'C';
//  printf("%c\n", foo[y][x].val);
    assign(foo, row, column, x, y, limit);

    for(int i = 0; i < row; i++)
    {
        for(int j = 0; j < column; j++)
        {

        //  foo[i][j].val = '.';
            printf("%d\t ", foo[i][j].filled);
        }
        printf("\n");
    }


}

void assign(elements ** elements, int row, int column, int x, int y, int limit)
{
    int tempX = x;
    int tempY = y;
    if(elements[y][x].filled != 0 )
    {
        //printf("reached.");
        return;
    }
    else if(limit < 0)
    {
        //printf("reached.");
        return;
    }
    else
    {
        if(elements[y][x].val != 'C')
            elements[y][x].val = limit + '0';
        elements[y][x].filled = true;

        tempX = x - 1;
        tempY = y;
        if (!( x < 0 || y < 0 || x > column - 1 || y > row -1 ))    
            assign(elements, row, column, tempX, tempY, limit - 1); // go up
        tempX = x;
        tempY = y + 1;
        if (!( x < 0 || y < 0 || x > column - 1 || y > row -1 ))    
            assign(elements, row, column, tempX, tempY, limit - 1); // go right
        tempX = x + 1;
        tempY = y;
        if (!( x < 0 || y < 0 || x > column - 1 || y > row -1 ))    
            assign(elements, row, column, tempX, tempY, limit - 1); // go down
        tempX = x;
        tempY = y - 1;
        if (!( x < 0 || y < 0 || x > column - 1 || y > row -1 ))    
            assign(elements, row, column, tempX, tempY, limit - 1); // go left
    }   
}

Are `row` and `column` different? Your allocation loop depends on them *not* being different. Can you please edit your question to show how you invoke the program (i.e. what arguments you pass)? — Some programmer dude, Mar 21 '17 at 08:15
`foo[i] = malloc( sizeof(elements) * row);` --> `foo[i] = malloc( sizeof(elements) * column);` **Side note**: You are not allocating a 2D array, [you are fragmentating your memory](http://stackoverflow.com/questions/7586702/is-2d-array-a-double-pointer) ;) — LPs, Mar 21 '17 at 08:17
@Someprogrammerdude What do you mean by row and column not being different? — bitnahian, Mar 21 '17 at 08:18
`for (int i = 0; i < column; i++) foo[i] = (elements *)malloc( sizeof(elements) * row);` --> `for (int i = 0; i < row; i++) foo[i] = (elements *)malloc( sizeof(elements) * column);` (already pointed out previous Q). — BLUEPIXY, Mar 21 '17 at 08:19
@BLUEPIXY I thought I was getting the error cause of not providing the correct argv[ ] arguments. Will try this out. — bitnahian, Mar 21 '17 at 08:20
You allocate `row` number of `elements*`, then you loop over `column` elements to allocate `row` number of `elements` structures. — Some programmer dude, Mar 21 '17 at 08:20
And if you don't provide the correct number of arguments you will have *undefined behavior* by going out of bounds of `argv`. You should *always* check `argc` to make sure you have the correct number of arguments. — Some programmer dude, Mar 21 '17 at 08:21
Lastly use a *debugger* to catch the crash "in action" and locate where it happens in your code. — Some programmer dude, Mar 21 '17 at 08:22
`if(elements[y][x].filled != 0 )` :note that `malloc` does not initialize members. — BLUEPIXY, Mar 21 '17 at 08:22
at `assign` First You need to check whether the values of x and y are within range. — BLUEPIXY, Mar 21 '17 at 08:26
`tempX = x - 1; /* at first time x is 0 */ tempY = y; /* y is 0 */ if (!( x < 0 || y < 0 || x > column - 1 || y > row -1 )) /* become true */ assign(elements, row, column, tempX /* as -1 */, tempY, limit - 1);` ==> `if(elements[y][x].filled != 0 )` ==> `if(elements[0][-1].filled != 0 )` bang!! — BLUEPIXY, Mar 21 '17 at 08:31
@BLUEPIXY I'm assuming that by default, boolean variables are set to false (i.e. 0) Still getting a segmentation fault. :( I haven't performed error checking yet. Was going to do it after it compiled with standard inputs. Also, how do I upvote comments? :O — bitnahian, Mar 21 '17 at 08:34
_how do I upvote comments?_ Click △ (Point the mouse to the left of the comment then click) — BLUEPIXY, Mar 21 '17 at 08:37
Best if you post a new question with corrected code and new problems. — LPs, Mar 21 '17 at 08:43
Guys, can you help me with the recursive function? :( It just ISN'T working. — bitnahian, Mar 21 '17 at 09:23
boolean values can have ANY value the code specifically has to initialize them to `true` or `false` — user3629249, Mar 21 '17 at 11:20
regarding this expression: `elements ** elements` it is a VERY poor programming practice to make a variable/parameter name the same as the struct/typedef name. While the compiler can/will figure it out as the two names are in different name spaces,via the context, for us humans there is no difference. This leads to lots of confusion. — user3629249, Mar 21 '17 at 11:24
when calling any of the `scanf()` family of functions, always check the returned value (not the parameter values) to assure the operation was successful. Similar to: `if( 1 != sscanf(argv[1], "%d", &row) ) { perror( "sscanf for first command line argument failed" ); exit( EXIT_FAILURE ); } // implied else, scanf successful` — user3629249, Mar 21 '17 at 11:32
the posted code is missing this critical code block: `if( 6 != argc ) { fprintf( stderr, "USAGE: %s \n", argv[[0] ); exit( EXIT_FAILURE ); }` — user3629249, Mar 21 '17 at 11:33
when calling any of the heap allocation functions: (malloc, calloc, realloc) 1) do not cast the returned value. The returned type is `void*` so can be assigned to any other pointer. Casting just clutters the code, making it much more difficult to understand, debug, maintain. 2) always check (!=NULL) the returned value to assure the operation was successful. — user3629249, Mar 21 '17 at 11:36
to avoid a memory leak, before exiting the program, always pass all allocated memory pointers to `free()` — user3629249, Mar 21 '17 at 11:37
need to check that `x` is less than `row` and `y` is less than `column` and exit the program with an appropriate error message (to stderr) if any of those two conditions are not true. — user3629249, Mar 21 '17 at 11:39
this line: `f(elements[y][x].filled != 0 )` is checking a field that has not been initialized This is undefined behavior. If you want all the fields to be initialized either use `calloc()` rather than `malloc()` or write a loop in the code that initializes all the fields — user3629249, Mar 21 '17 at 11:42
this line: `else if(limit < 0)` should have been done back in `main()` right after inputting the command line parameters. — user3629249, Mar 21 '17 at 11:43
since the array has not been initialized, this line: `if(elements[y][x].val != 'C')` will always be true because it was set back in `main()` — user3629249, Mar 21 '17 at 11:49
in general, the first [] in an array is considered the row number and the second [] set is considered the column number so this line: `if (!( x < 0 || y < 0 || x > column - 1 || y > row -1 ))` and similar lines are nonsense. Note: each of the recursive calls to `assign()` can pass parameters that are outside the bounds of the array. This results in undefined behavior and can lead to a seg fault event. — user3629249, Mar 21 '17 at 11:55
@user3629249 Thank you so much. I'd lost all hope. Will try it out :) — bitnahian, Mar 22 '17 at 11:09

score 0 · Answer 1 · answered Mar 21 '17 at 12:06

each of the if() code blocks in the last half of assign() are beginning with the same basic parameter values, except the limit changes.

so the total number of recursions (which the code seems to be limiting the the value in 'limit' is not actually limited,

because when limit is 0, it is not necessarily the last call to be made to assign() and once limit is <0 the code will recurse about 16 gig more times. (at least) This is probably why the program crashes

Suggest decrementing limit within assign() before any of the recursive calls to assign()

Segmentation fault on trying to pass a recursive function to populate a 2D array

1 Answers1