-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

DevSolar
  • 67,862
  • 21
  • 134
  • 209
  • 1
    Please do not post your code as images. Include it directly in the question and use proper formatting. Use the 'Edit' link to edit your question. The the [ask] page for more information. – fredrik Jan 15 '21 at 10:38
  • Take inspiration from *existing* open source software on [github](http://github.com/). Be aware of [ANSI escape code](https://en.wikipedia.org/wiki/ANSI_escape_code). Consider using [ncurses](https://en.wikipedia.org/wiki/Ncurses) or [GTK](http://gtk.org/). Read documentation of your C compiler (e.g. [GCC](http://gcc.gnu.org/), to be invoked as `gcc -Wall -Wextra -g`) and of your debugger (e.g. [GDB](https://www.gnu.org/software/gdb/)). Consider installing [Debian](http://debian.org/) on your laptop – Basile Starynkevitch Jan 15 '21 at 10:40
  • 1
    sizeof( char ) is, by definition, one. [Don't cast the result of malloc().](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – DevSolar Jan 15 '21 at 10:42
  • 5
    the two codes are identical. `i <= 16`must be `i < 16`; off-by one error. We don't cast the return value of `malloc` in C. And `*(grid + i)` is eyesore, should be written `grid[i]` – Antti Haapala -- Слава Україні Jan 15 '21 at 10:43
  • The usual idiom is `grid = malloc(16 * sizeof *grid)`. Do not cast the return, and reference the sizeof the object rather than the type. This allows the type to be modified without needing to change the allocation and allows the reader to verify the call without needing to reference the declaration. – William Pursell Jan 15 '21 at 10:46
  • 2
    The reason given for the closing makes no sense, though. The question itself is perfectly valid. I am sure we have an appropriate duplicate somewhere. – DevSolar Jan 15 '21 at 10:47
  • Please use a [valid prototype for `main`](https://stackoverflow.com/questions/2108192/what-are-the-valid-signatures-for-cs-main-function). – costaparas Jan 15 '21 at 10:47
  • 1
    @Mahdi: All the comments here have valid improvements for your code. The actual error is the `<=` in the loop -- you allocate memory for 16 elements, then you loop over indices 0 through 16, which makes *17* loops, which results in undefined behavior and your program acting "funny". -- The correct idiom is to allocate "size", and loop `for ( i = 0; i < "size"; ++i )` (note the `<`, not `<=`). – DevSolar Jan 15 '21 at 10:50
  • @DevSolar and everybody thank you. that's the exact problem and now it works – Mahdi Chaari Jan 15 '21 at 10:52
  • 1
    I voted to close for details and clarity - which has now been added with the code, as such I've also voted for reopen. What the other two voted for I can't say - but judging from the message given in the close notice - it was something else. – fredrik Jan 15 '21 at 10:52

1 Answers1

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.

DevSolar
  • 67,862
  • 21
  • 134
  • 209
  • 1
    Program now works perfectly. but I don't understand why our teacher insists on using casting whereas pretty much everybody advices not to do so. – Mahdi Chaari Jan 15 '21 at 11:39
  • @MahdiChaari It's a different thing in C++ (where, however, you should not malloc(), or even new(), manually in the first time), and not many casual C programmers realize the difference. In C it is not exactly *wrong* either (if you cast to the *correcr* type), it is just suboptimal. – DevSolar Jan 15 '21 at 15:21