-1

I'm writing a solution to the 'flea problem' on Project Euler (https://projecteuler.net/problem=213). The 'movement' of the 'fleas' and all that works fine. However, after a few turns (different amount every time) the array shrink to half of the original size (30 to 15 or 16, 20 to 10 or 11).

Any help would be appreciated.

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

int main() {
//https://projecteuler.net/problem=213

int grid[30][30];
int size = sizeof(grid) / sizeof(grid[0]);

for(int x = 0; x < size; x++) { //Setting the fleas down.
    for (int y = 0; y < size; y++) {
        grid[x][y] = 1;
    }
}

srand(time(NULL)); //Setting the random stuff

for(int turn = 1; turn <= 50; turn++) {
    for(int x = 0; x < size; x++) { //Showing the full grid every turn.
        for (int y = 0; y < size; y++) {
            printf("%d", grid[x][y]);
        }
        printf("\n");
    }

    printf("\n\n\n");

    for(int x = 0; x < size; x++) {
        for(int y = 0; y < size; y++) {
            for(int flea = 0; flea < grid[x][y]; flea++) { //Moving the fleas.
                grid[x][y]--; //Taking them off of their cell.

                int direction = rand() % 4; // 3 is up, 2 is down, 1 is left, 0 is right

                while (x == 0 && y == 0 && (direction == 3 || direction == 1)) { //Grab a new direction if it's at the corner and wants to go off.
                    direction = rand() % 4;
                }
                while (x == 0 && y == size && (direction == 3 || direction == 0)) {
                    direction = rand() % 4;
                }
                while (x == size && y == 0 && (direction == 2 || direction == 1)) {
                    direction = rand() % 4;
                }
                while (x == size && y == size && (direction == 2 || direction == 0)) {
                    direction = rand() % 4;
                }

                while (x == 0 && direction == 3) { //Same, but for the edge.
                    direction = rand() % 4;
                }
                while (x == size && direction == 2) {
                    direction = rand() % 4;
                }
                while (y == 0 && direction == 1) {
                    direction = rand() % 4;
                }
                while (y == size && direction == 0) {
                    direction = rand() % 4;
                }

                switch (direction) {
                    case 3: //Up
                        grid[x][y - 1]++;

                        break;

                    case 2: //Down
                        grid[x][y + 1]++;

                        break;

                    case 1: //Left
                        grid[x - 1][y]++;

                        break;

                    case 0: //Right
                        grid[x + 1][y]++;

                        break;
                }
            }
        }
    }
}

int empty = 0;

for (int x = 0; x < size; x++) { //Counting the empties.
    for (int y = 0; y < size; y++) {
        if(grid[x][y] == 0) {
            empty++;
        }
    }
}

printf("%d", empty);

return 0;
}
  • 1
    What do you mean the array shrinks? An array can't change its size. – Barmar Oct 20 '20 at 01:46
  • *"the array shrink to half of the original size"* - Nope – klutt Oct 20 '20 at 01:47
  • 4
    I suspect you're writing outside the array, and overwriting the `size` variable as a result. – Barmar Oct 20 '20 at 01:48
  • 1
    Print the seed value passed to `srand()`, and make arrangements to be able to specify the seed (e.g. command line arguments). Record when the program fails; rerun the program with the same seed and ensure it still fails. Now you have a reproducible problem, you can track through the code with a debugger or extensive printing statements. The chances are you have some sort of 'out of bounds' memory access. – Jonathan Leffler Oct 20 '20 at 01:49
  • My guess is that the `while` loops that are supposed to keep you from going outside the grid aren't right. – Barmar Oct 20 '20 at 01:50
  • @Barmar For some reason, the array goes from 30 by 30 to 15 by 15 (or 16 by 16). Printing 'size' every turn shows me that it's going from 30 to 15. Also, printing the array shows me that halfway through it starts printing a 15 by 15 array. –  Oct 20 '20 at 01:50
  • That doesn't mean the array is shrinking, it means that the `size` variable is changing. – Barmar Oct 20 '20 at 01:51
  • 3
    `x == size` should be `x == size - 1`, same with `y` – Barmar Oct 20 '20 at 01:52
  • 1
    If by "array shrinks" you mean that the variable `size` is unexpectedly changing, then you should run your program line by line in a [debugger](https://stackoverflow.com/q/25385173/12149471), to see at what point this variable gets modified. As has already been stated by someone else, it is possibly being modified by an out of bounds write to the array. With the debugger, you will be able to find out which line in your code is responsible. – Andreas Wenzel Oct 20 '20 at 01:53
  • 1
    Your "flea" updates using `switch (direction)` are not being bounds-tested and you're gonna stomp all over your program's memory. – paddy Oct 20 '20 at 01:58
  • @paddy The loops like `while (y == 0 && direction == 1)` are the bounds tests. He just has a fencepost error in some of them. – Barmar Oct 20 '20 at 02:10
  • It's hard to call those bounds-tests, when they are handling `x` and `y` independently and re-rolling the `direction` each time. Even if the test was corrected to use `size - 1` instead of `size`, it would still be an issue, because a point in any corner might re-roll direction on its y-test to go out of bounds in `x`. – paddy Oct 20 '20 at 02:17

3 Answers3

3

The reported symptom was that the value of the size variable sometimes changes. This does not mean that the grid array is changing size. What it means is that there's a bug in the program that results in the size variable being overwritten. This is the result of incorrect bounds checks when writing to grid.

The bounds checks to see if the direction would take the flea off of the grid aren't right. Each of the while loops tests a single condition, generating a new direction until the direction satisfies that condition.

However, after a condition is satisfied, it is never checked again. If a later condition fails, and a new direction is chosen, is may violate any of the previous conditions without being detected. As a result, it may write past the bounds of the grid, causing memory corruption.

In addition, some of the edge tests are incorrect (they are checking the wrong edge for a given direction). Also, it's comparing x and y to size instead of size - 1.

This can be simplified and corrected as follows: First, there is no reason for 8 separate tests. The corners do not need to be special cases. It just needs to check the edges. Second, there needs to be a single loop that executes until all conditions are met. It can look like this:

int direction;
while (true) {
    direction = rand() % 4;
    if (direction == 0 && x == size - 1) continue;
    if (direction == 1 && x == 0) continue;
    if (direction == 2 && y == size - 1) continue;
    if (direction == 3 && y == 0) continue;
    break;
}

That's all that's needed to handle all of the edge conditions. If any condition fails, a continue statement is executed, which causes the loop to resume at the top. The loop only exits if all conditions are met and the break statement is executed. The key point is that when a new random number is chosen, all of the conditions are checked.

An alternative approach would be to instead construct an array of the valid directions, then choose one of the directions from the array, knowing in advance that it's valid. This has the advantage that rand() never has to be called more than once. The array would have length 4, but there would be either 2, 3, or 4 valid entries per use (2 for corners, 3 for non-corner edges, and 4 for non-corner, non-edge cells).

Tom Karzes
  • 22,815
  • 2
  • 22
  • 41
  • Worth mentioning the UB in `for(int x = 0; ...` and `grid[x - 1][y]++;` with the same for `y`. – David C. Rankin Oct 20 '20 at 05:06
  • @DavidC.Rankin Yes, that's a problem if the edge checks aren't working properly, as in the original posted code. Those `grid` updates are where the memory corruption is coming from. With the proper checks in place, they will work properly. – Tom Karzes Oct 20 '20 at 05:14
  • I figured that was the case, but didn't follow the snippets all the way through. You got my nod for a good answer. (probably the array trying to scratch all those fleas that mucked the stack up... `:)` – David C. Rankin Oct 20 '20 at 05:20
2

You're accessing invalid memory. From address sanitizer:

==10604==ERROR: AddressSanitizer: stack-buffer-underflow on address 0x7ffc433d0f14 at pc 0x55c6134daa01 bp 0x7ffc433d0eb0 sp 0x7ffc433d0ea0
READ of size 4 at 0x7ffc433d0f14 thread T0
    #0 0x55c6134daa00 in main /tmp/so/as.cpp:74
    #1 0x7fb3beae9d0a in __libc_start_main ../csu/libc-start.c:308
    #2 0x55c6134da159 in _start (/tmp/so/as+0x1159)

Address 0x7ffc433d0f14 is located in stack of thread T0 at offset 20 in frame
    #0 0x55c6134da224 in main /tmp/so/as.cpp:5

Line 74 matches with your left case:

                    case 1: //Left
                        grid[x - 1][y]++;

                        break;

Since asan aborts on the first address issue, which in this case is access out of bounds memory. On the same line though, you try to write to an invalid address--at some point you'll end up trampling memory.

You have the right idea with your traps, but a few of them have buggy logic (in the case of left, you need to check for x == 0 and you're actually checking y == 0).

Stephen Newell
  • 7,330
  • 1
  • 24
  • 28
0

other posters have diagnosed the issue; I suggest it would be easier to understand if the direction validation code was embedded into the state update code:

for(;;) {
  switch(rand() % 4) {
    case 0:
      if (y == 0) continue;
      grid[x][y - 1]++;
      break;
    case 1:
      if (y == size - 1) continue;
      grid[x][y + 1]++;
      break;
    case 2:
      if (x == 0) continue;
      grid[x - 1][y]++;
      break;
    case 3:
      if (x == size - 1) continue;
      grid[x + 1][y]++;
      break;
    }
    break;
}

or a goto might make it more obvious what's going on!

I also note that you're only moving half the fleas. your loop does:

            for(int flea = 0; flea < grid[x][y]; flea++) { //Moving the fleas.
                grid[x][y]--; //Taking them off of their cell.

when it should probably do:

            while(grid[x][y] > 0) { //Moving the fleas.
                grid[x][y]--; //Taking them off of their cell.
Sam Mason
  • 15,216
  • 1
  • 41
  • 60