1
typedef struct Cell {
    float   altitude;
    int     type;
}Cell;

void MAZE(FILE *fp, Cell *Map);

int main(void) {

    FILE *fp = fopen("map.bin", "rb");
    Cell *Map;
    Map = read_file(fp);
    char choice;
    while (1) {
        system("color 0f");
        system("cls");
        puts("Main menu:");
        puts("1. Show map by type.");
        puts("2. Show map by altitude.");
        puts("3. Build route.");
        puts("4. Find suitable places for biker jumps.");
        puts("5. Quit.");
        std::cin >> choice;
        std::cin.ignore();
        switch (choice) {
        case '1': {
            display_map_by_type(fp, Map);
            continue;
        }
        case '2': {
            display_map_by_altitude(fp, Map);
            continue;
        }
        case '3': {
            MAZE(fp, Map);
            continue;
        }
        case '5': {
            puts("You've decided to quit.");
            free(Map);
            return 0;
        }
        default: {
            system("color 9f");
            puts("Invalid choice.");
            puts(Press);
            getche();
            continue;
        }
        }
        return 0;
    }
}
void MAZE(FILE *fp, Cell *Map) {
    bool (*initial_maze)[10][10] = (bool (*)[10][10])malloc(sizeof(bool[10][10]));
    if (initial_maze == NULL) {
        fprintf(stderr, NaM);
        exit(-3);
    }
    for (int i = 0; i < 10; i++) {
        for (int j = 0; j < 10; j++) {
            if (Map[i * 10 + j].type == 2 || Map[i * 10 + j].type == 4) {
                *initial_maze[i][j] = false;
            }
            else {
                *initial_maze[i][j] = true;
            }
        }
    }
    for (int i = 0; i < 10; i++) {
        for (int j = 0; j < 10; j++) {
            printf("[%d]", Map[i *10 + j].type);
        }
        putchar('\n');
    }
    putchar('\n');
    for (int i = 0; i < 10; i++){
        for (int j = 0; j < 10; j++) {
            printf("%3s", *initial_maze[i][j] ? "." : "[]");
        }
        putchar('\n');
    }
    getchar();
    free(initial_maze);
}

This program has an array of objects loaded into the heap - Cell *Map. The array is available in the main and is passable and free()-able in the main or any other function. Yet for some reason there's a segfault when choosing option 3. Why the segfault occurs (both with Cell *Map and bool *initial_maze[10][10]) is unknown to me. At least I don't find any reason for a segfault, no uninitialized memory is accessed, no multiple free()-s. Technically there is a free() in the switch which is in a loop, but that still can't be it, because right after that there is a return statement. To top it off it doesn't always crash either, although it does in most cases.

The debuggers in Visual Studio and Code Blocks pointed at the free(initial_maze); line, but there's nothing wring with it, there's nothing that could have invalidated the array before that.

For some odd reason Visual Studio also pointed at the system("color 0f"); line as if it could trigger an exception.

As for some context, what's happening in the void MAZE(FILE *fp, Cell *Map); function. It's supposed to solve a maze, but first it has to create a 2d bool array based on the 1d array of objects passed to it.

I've read the documentation, I'm familiar with the rules when using dynamical memory.

If you want some output:

[0][0][2][3][1][4][1][2][0][3]
[2][1][1][0][3][4][0][2][1][0]
[4][0][4][3][1][1][0][4][1][2]
[2][1][3][1][3][2][1][3][1][0]
[2][2][3][0][0][1][2][4][4][3]
[1][3][2][1][2][2][0][1][1][3]
[4][4][0][3][1][2][0][4][1][2]
[3][0][4][4][4][1][0][3][2][0]
[1][4][3][4][3][4][0][1][1][0]
[1][1][2][2][1][1][3][1][3][3]

  .  . []  .  . []  . []  .  .
 []  .  .  .  . []  . []  .  .
 []  . []  .  .  .  . []  . []
 []  .  .  .  . []  .  .  .  .
 [] []  .  .  .  . [] [] []  .
  .  . []  . [] []  .  .  .  .
 [] []  .  .  . []  . []  . []
  .  . [] [] []  .  .  . []  .
  . []  . []  . []  .  .  .  .
  .  . [] []  .  .  .  .  .  .

and then the program hangs for a bit and crashes.

halfer
  • 19,824
  • 17
  • 99
  • 186
  • 1
    Start by learning how to use a *debugger* to catch crashes as and when they happen. Then you can use it to locate where in your code it happens, and also examine all involved variables to make sure they are all alright. – Some programmer dude Oct 12 '18 at 09:42
  • 1
    Oh and you should learn about [*operator precedence*](http://en.cppreference.com/w/c/language/operator_precedence), and think about it when you use the dereference operator `*` in the `MAZE` function. – Some programmer dude Oct 12 '18 at 09:46
  • 1
    To that point, I see *zero* reason for dynamic allocation of `initial_maze` in this code in the first place. – WhozCraig Oct 12 '18 at 09:46
  • @WhozCraig Even if I move `initial_maze` to the stack that's only treating a symptom. Apparently my logic is flawed somewhere. – Larry Teischwilly Oct 12 '18 at 09:48
  • The question is now re-tagged to the programming language used. Please make an effort of knowing which programming language you are coding in. I've added a remark to my answer, since I posted it before spotting the `cin`. – Lundin Oct 12 '18 at 09:54
  • @LarryTeischwilly my point was, the operator precedence problem mentioned by Some programmer dude disappears when you no longer have to do it (use a leading `*`) it in the first place once the unnecessary dynamic allocation is removed. – WhozCraig Oct 12 '18 at 09:54

1 Answers1

2

Disclaimer: This answer is for C, since you had tagged the question as such. However, your code is actually C++.


It is a simple operator precedence bug, *initial_maze[i][j] should be (*initial_maze)[i][j]. Since [] has higher precedence than *, you end up referencing an array bool[10][10] times i, which wasn't the intention.

But instead of doing that, change the malloc call to this:

bool (*initial_maze)[10] = malloc(sizeof(bool[10][10]));

And then access it as initial_maze[i][j]. Much easier to read.

Though as mentioned in the comments, there is little need for dynamic memory in the first place. You could just make a local 2D array.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • 3
    I concur, but regardless of the question tags, believe it or not, this code is being compiled with a C++ toolchain. Either that or the OP has the most forgiving C compiler that accept things like `std::cin >> choice;`. That's why the hard cast was on the (ill-advised) malloc call. – WhozCraig Oct 12 '18 at 09:51
  • @WhozCraig Gah, it needs to be retagged. Regardless, just swap the malloc call for `new bool[10][10]` or `std::vector`. – Lundin Oct 12 '18 at 09:52
  • Well goddamn, this was simple. I'm aware `cin` is part of C++ but aside from that and the cast I don't use anything that's part of C++. I chose `cin` because using `scanf` in a loop and making sure it hadn't put an accidental `'\n'` in `stdin` is too much of a headache. – Larry Teischwilly Oct 12 '18 at 09:53
  • 3
    @LarryTeischwilly Compiling the code as C++ makes all the difference though. There's many subtle differences. The `bool` type behaves differently, `malloc` is strongly discouraged in C++, C++ does not have VLA and so on. – Lundin Oct 12 '18 at 09:57
  • Indeed, C vs C++ is not which features you used, but which compiler you used. Although common advice of course is to use the right tool for the job :P – Lightness Races in Orbit Oct 12 '18 at 10:21