system("cls") in C is showing some weird behavior after using dynamic allocation

-1

I've just started learning C and I decided to create a snake games using characters.

So I started building blocks for the games (functions and all). And I try to test each block individually.

So after some time I created the movement block and tested it. the program returned 0xC0000005 which appears to be illegal memory access error code.

After some tinkering I found that the problem is with the system("cls") function. I experimented with putting it elsewhere in the program and this behavior emerged:

If I use dynamic allocation the system("cls") no longer works.

This code works fine because the system("cls") is used before dynamic allocation:

#include <stdio.h>

main()
{
    char ** grid;
    int i;

    system( "cls" );
    grid = (char**)malloc( 16 * sizeof( char * ) );
    for ( i = 0; i <= 16; ++i )
    {
        *(grid + i) = (char*)malloc( 75 * sizeof( char ) );
    }
}

Whereas this code returns an error because it is called after the dynamic allocation:

#include <stdio.h>

main()
{
    char ** grid;
    int i;

    grid = (char**)malloc( 16 * sizeof( char * ) );
    for ( i = 0; i <= 16; ++i )
    {
        *(grid + i) = (char*)malloc( 75 * sizeof( char ) );
    }
    system( "cls" );
}

EDIT: after a bit of tinkering I found that reducing the size of each pointer allocated memory solves the problem which makes no sense

c
asked on Stack Overflow Jan 15, 2021 by Mahdi Chaari • edited Jan 15, 2021 by DevSolar

1 Answer

2
grid = (char**)malloc( 16 * sizeof( char * ) );
for ( i = 0; i <= 16; ++i )

You reserve space for 16 elements. Then you loop over the indices 0 through 16, including the 16 -- which makes for 17 loops, one too many. This leads to undefined behavior and your program acting funny.

The correct idiom is to loop to size, exclusive:

grid = (char**)malloc( 16 * sizeof( char * ) );
for ( i = 0; i < 16; ++i )

Note the < instead of <=.


As for various other issues with your code, here is a stylistically cleaned up version for you to consider:

// stdio.h was unused, but these two were missing
#include <string.h>            // for malloc()
#include <stdlib.h>            // for system(), but see below

// ncurses really is the go-to solution for console applications
#include <curses.h>

// Define constants globally instead of having "magic numbers" in the source
// (and then forgetting to adjust one of them when the number changes)
#define GRID_ROWS 16
#define GRID_COLUMNS 75

// int main( void ) or int main( int argc, char * argv[] )
int main( void )
{
    char ** grid;

    // system() is pretty evil. There are almost always better solutions.
    // For console applications, try Ncurses library.
    //system( "cls" );
    clear();

    // don't cast malloc(), sizeof object instead of type, using constant
    grid = malloc( GRID_ROWS * sizeof( *grid ) );

    // loop-scope index, using constant
    for ( int i = 0; i < GRID_ROWS; ++i )
    {
        // indexed access, no cast, sizeof( char ) == 1, using constant
        grid[i] = malloc( GRID_COLUMNS );
    }

    // You should also release any resources you allocated.
    // Relying on the OS to do it for you is a bad habit, as not all OS
    // actually can do this. Yes, this makes your program more complex,
    // but it also gives you good hints which parts of main() should
    // be made into functions, to ease clean-up.

    // return 0; from main() is implicit, but still a good habit
    // to write out.
    return 0;
}

The wrong includes hint at you not using, or not paying attention to, compiler warnings. Something like -Wall -Wextra is the bare minimum of warning settings (there are a lot more available, check your compiler manual). There is also something like -Werror which interprets any warning as a hard error; it might be instructive to use that while you are still learning your way around the language.

answered on Stack Overflow Jan 15, 2021 by DevSolar • edited Jan 15, 2021 by DevSolar

User contributions licensed under CC BY-SA 3.0