Thread: A snake game - Memory problem

Your "allocate" is absolutely broken.

You are not using the "new" pointer returned by realloc. Even if snake was modified inside Allocate, it will not modify the snake pointer passed in, and thus the calling code's snake will remain the same one it was. Look at how my realloc does things: It doesn't modify the original pointer (but it does free it - although I just realized I'd missed that bit - I'll go back and edit it).

Likewise for MoveAndDraw - it can not change the snake outside of this function.

To modify ANYTHING passed into a function, you must have a pointer to whatever you are mdifying - like the size you are passing to move. To modify a pointer to int to point elsewhere, you need a pointer to pointer to int.

--
Mats

You don't seem to get it. Both realloc and especially malloc will give you back a new memory address. If you look again at my demo example where I've used int's, this is exactly what you're doing with pointers. It's like you expect bar to be 2 at the end of main. I've tried explaining that it simply doesn't work that way. You can't update the pointer value just by passing the pointer, just like you can't update an int just by passing an int.
I'm not just giving you idea on ways you could make it work. I'm telling you the only ways there are to make it work. The only choice besides what I've been suggesting so far is to use a global, and update that directly without passing anything. The reason I didn't suggest that earlier is because that way is considered to be bad, but it's otherwise easier than the other two ways. That's it, those really are your only 3 choices.
It seems like you are determined to find another (easier) way. But there just isn't another way in C. If you don't follow what I've been telling you, then it cannot ever work, period. The easy way is to learn how to manage dynamic memory allocation now, so that next time you will already have the knowledge.

Actually your use of realloc just makes things even worse! Now you're passing the same pointer into realloc every time regardless of what realloc returned last time, so in addition to the buffer overrunning you've been doing, you're now potentially corrupting the heap in other ways. There are reasons why C++ doesn't have the equivalent of realloc. realloc just isn't as useful as you'd think. It's a lot of complexity for very little gain, and all to often, people don't use it right. Treat it as for experts only, meaning you can forget it even exists for a least a few more years.

So I see you've had a long thread where you wrote a snake game using a grid. That code looked really long and complicated! What I've been suggesting should in fact be shorter than what you have posted so far in this thread. In fact, in the time I've spent posting to this thread I could have written the whole program already. But I'll certainly forget trying to suggest doing it that way now as I can see you've been put off it or something.

Thanks again (:

Code:

void Allocate(LOC** snake, int size)
{
     *snake = (LOC*)realloc(snake,sizeof(LOC)*size);
     if(*snake == NULL)
     {
              printf("Allocating Error");
              Clear(*snake);
              getch();
              exit(1);
     }
}

Code:

Allocate(&snake,++(*size));

What am I missing now? \=
Can you give me just the fixed code please? (;
I'll undersand better my mistakes by looking at the right code..

Nice, you're getting the idea now! You've right, a working solution might be good about now.
In order to get the data across from the old piece of memory to the new one, we actually also need to know how big the old one was too. Here's one way to do it, using malloc:

Code:

void Allocate(LOC** snake, int oldSize, int newSize)
{
    // In case you want to be able to call this whenever and have it only do stuff when necessary:
    if (oldSize == newSize)
        return;
    // Try allocating a new piece of memory for the snake
    LOC *newSnake = malloc(snake, sizeof(LOC)*newSize);
    if (newSnake == NULL)
    {
        printf("Failed to allocate a new snake");
        // Do whatever you like here I suppose...
        Clear(*snake);
        *snake = NULL;
        getch();
        exit(1);
    }
    // Copy the old data over to the new piece of memory
    memcpy(newSnake, *snake, oldSize);
    if (newSize > oldSize) // Clear out the new bit perhaps
        ZeroMemory(newSnake + oldSize, sizeof(LOC)*(newSize-oldSize));
    // All set, free the old piece of memory, and copy the new pointer across.
    free(*snake);
    *snake = newSnake;
}

Nice nice but it doesn't work \=
Why do I need this "newSnake"?

Originally Posted by gavra

Nice nice but it doesn't work \=
Why do I need this "newSnake"?

because realloc could fail and you will miss the original pointer

Originally Posted by vart

because realloc could fail and you will miss the original pointer

Indeed, you will miss it dearly because you would then be unable to free the memory that it pointed to.

Why do I miss the original pointer?

Anyway, it doesn't work when I eate the first food.. (why should it fail so early?)

Originally Posted by gavra

Why do I miss the original pointer?

Anyway, it doesn't work when I eate the first food.. (why should it fail so early?)

Don't know why it's failing.

You need the original pointer in case realloc returns NULL because it can't allocate more memory. In that case, the original memory is not freed, and you have a memory leak.

--
Mats

\=
Well, I don't think that realloc fails in my case (I mean if everything is right with the code..).
I guess that if you don't know I may go to find out the answer aomewhere else \:

Thanks everyone (:

Originally Posted by gavra

Well, I don't think that realloc fails in my case (I mean if everything is right with the code..).

Even if your code is perfect, realloc() will fail if there is no block of memory available that is large enough to be allocated as requested.

Originally Posted by gavra

I guess that if you don't know I may go to find out the answer aomewhere else \:

If you want help, post your current code. Clearly, no one other than yourself can know exactly why you have a bug when we do not know what is your current code.

I have already post the changes but if you want:

Code:

#include <stdio.h>
#include <conio.h>
#include <time.h>
#include <stdlib.h>
#include <User/Console.h>

#define UP 72
#define LEFT 75
#define RIGHT 77
#define DOWN 80
#define ESC 27

typedef struct
{
        int x;
        int y;
        int exist;
}INFO;

typedef struct
{
        int x;
        int y;
}LOC;

void Clear(LOC* snake)
{
     free(snake);
}

void Initialize(LOC snake[], int* size, INFO* food, int* direction)
{
     int i;
     srand((unsigned)time(NULL));
     ResizeConsoleWindow(50,30);
     ChangeConsoleTitle("Snake");
     HideConsoleCursor();
     *size = 3;
     *direction = LEFT;
     for (i=0;i<*size;i++)
     {
         snake[i].x = 40 + i;
         snake[i].y = 10;
     }
     food->exist = 0;
}

void Input(int *input, LOC* snake)
{
     int temp = *input;
     if (kbhit())
        *input = getch();
     if (*input == 224)
     {
        *input = getch();
        switch(*input)
        {
                      case UP: if (temp == DOWN) *input = temp;
                           break;
                      case DOWN: if (temp == UP) *input = temp;
                           break;
                      case LEFT: if (temp == RIGHT) *input = temp;
                           break;
                      case RIGHT: if (temp == LEFT) *input = temp;
                           break;
        }
     }
     else if (*input == ESC)
          {
             Clear(snake);
             exit(0);
          }
     else *input = temp;
}

void Allocate(LOC** snake, int size)
{
     *snake = (LOC*)realloc(snake,sizeof(LOC)*size);
     if(*snake == NULL)
     {
              printf("Allocating Error");
              Clear(*snake);
              getch();
              exit(1);
     }
}

void MoveAndDraw(LOC snake[], int *size, int direction, INFO* food)
{
     int i;
     if ( (snake[0].x == food->x && snake[0].y == food->y) )
     {
         Allocate(&snake,++(*size));
         snake[*size-1].x = snake[*size-2].x;
         snake[*size-1].y = snake[*size-2].y;
         FilledCircle(food->x*8,food->y*12.33,4,BLACKRGB);
         food->exist = 0;
     }
     if(food->exist)
        FilledCircle(food->x*8,food->y*12.33,4,REDRGB);
     Circle(snake[*size-1].x*8,snake[*size-1].y*12.33,5,BLACKRGB);
     for (i=*size-1;i>0;i--)
     {
         snake[i].x = snake[i-1].x;
         snake[i].y = snake[i-1].y;
     }
     Circle(snake[0].x*8,snake[0].y*12.33,5,YELLOWRGB);
     switch (direction)
     {
            case UP: snake[0].y--;
                 break;
            case DOWN: snake[0].y++;
                 break;
            case LEFT: snake[0].x--;
                 break;
            case RIGHT: snake[0].x++;
                 break;
     }
     Circle(snake[0].x*8,snake[0].y*12.33,5,GREENRGB);
}

void CreateFood(LOC snake[], int size, INFO* food)
{
     int x, y, i;
     RANDOM:
     x = rand()%49+1; y = rand()%29+1;
     for (i=0;i<size;i++)
         if (snake[i].x == x && snake[i].y == y)
            goto RANDOM;
     food->x = x;
     food->y = y;
     food->exist = 1;
     FilledCircle(food->x*8,food->y*12.33,4,REDRGB);
}

int Crash(LOC snake[], int size)
{
    int i;
    for (i=1;i<size;i++)
        if (snake[0].x == snake[i].x && snake[0].y == snake[i].y)
           return 1;
    if (snake[0].x == 0 || snake[0].y == 0 || snake[0].x == 51 || snake[0].y == 31)
       return 1;
    return 0;
}

void End(LOC* snake, int size)
{
     Clear(snake);
     ClearConsole();
     gotoxy(10,10);
     SetAttColor(LIGHTBLUE);
     printf("Final Lenght: %d", size);
     getch();
}

int main()
{
    int size, direction;
    LOC *snake = malloc(sizeof(LOC)*3);
    INFO food;
    Initialize(snake, &size, &food, &direction);
    while(!Crash(snake, size))
    {
           Input (&direction, snake);
           if (!food.exist)
              CreateFood(snake, size, &food);
           MoveAndDraw(snake, &size, direction, &food);
           Sleep(((70-(size/3))>0)*(70-(size/3)));
    }
    End(snake, size);
    return 0;
}

Remember I also mentioned that the prototype of this function would need to change?:

Code:

void MoveAndDraw(LOC snake[], int *size, int direction, INFO* food)

Here's it changed to match the method you have chosen:

Code:

void MoveAndDraw(LOC **snake, int *size, int direction, INFO* food)

Now, can you see how you'd have to do the same thing as Allocate? And can you see that it needs to be done in this function as well, for exactly the same reasons?

It's not a case of we can't help you. It's a case of we've given you the necessary information but you seem to often be disregarding what we say. That's because you come half-way towards the solution and coming half-way doesn't work so you go all the way back again. If you'd just go the other half you'd be done.

\=
humm but it's more comfortable to access the snake as an array..

Originally Posted by gavra

\=
humm but it's more comfortable to access the snake as an array..

You can always make a copy of the incoming parameter, e.g.

Code:

void MoveAndDraw(LOC **pSnake, int *size, int direction, INFO* food)
{
   LOC *snake = *pSnake;

If you do that, you only need pSnake when you actually want to modify the pointer to the snake [and of course, you'll need to re-load the snake after it's been modified, otherwise you'll be reading the "old" snake].

--
Mats