1

Trying to make a 2D array and am getting this error from valgrind 

==226== HEAP SUMMARY:
==226==     in use at exit: 0 bytes in 0 blocks
==226==   total heap usage: 38 allocs, 38 frees, 9,793 bytes allocated
==226== 
==226== All heap blocks were freed -- no leaks are possible
==226== 
==226== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
==226== 
==226== 1 errors in context 1 of 2:
==226== Invalid read of size 8
==226==    at 0x108BB7: freeBoard (Battleships.c:55)
==226==    by 0x108B81: createBoard (Battleships.c:47)
==226==    by 0x108AD6: main (Battleships.c:30)
==226==  Address 0x522ff60 is 0 bytes after a block of size 32 alloc'd
==226==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==226==    by 0x108B29: createBoard (Battleships.c:40)
==226==    by 0x108AD6: main (Battleships.c:30)
==226== 
==226== 
==226== 1 errors in context 2 of 2:
==226== Invalid write of size 8
==226==    at 0x108B5D: createBoard (Battleships.c:44)
==226==    by 0x108AD6: main (Battleships.c:30)
==226==  Address 0x522ff60 is 0 bytes after a block of size 32 alloc'd
==226==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==226==    by 0x108B29: createBoard (Battleships.c:40)
==226==    by 0x108AD6: main (Battleships.c:30)
==226== 
==226== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

This is the code that is allocating/freeing

void createBoard(int* widthPtr, int* heightPtr)                                             /* Creates the 2D Board Array. */
{
    char** board;
    int i;
    board = (char**)malloc((*heightPtr) * sizeof(char*));

    for (i = 0; i < *widthPtr; i++)
    {
        board[i] = (char*)malloc((*widthPtr) * sizeof(char));
    }

    freeBoard(board, widthPtr);
}

void freeBoard(char** board, int* widthPtr)
{
    int i;
    for (i = 0; i < *widthPtr; i++)
    {
        free(board[i]);
    }
    free(board);
}

I'm trying to make a 2D array of a specified width/height which are integers. In each section of the array a char will be stored.

Any help would be great, thanks.

Argon
  • 55
  • 1
  • 7
  • 2
    Consider using 2D arrays instead: [Correctly allocating multi-dimensional arrays](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays). – Lundin Apr 14 '20 at 08:51
  • the error messages show specific line numbers, however, you have not indicated which lines in your posted code match those line numbers. Also, the error messages indicate the path back to `main()` but you have not posted the function: `main()` Please post a [mcve] so we can reproduce the problem and help you debug it. – user3629249 Apr 15 '20 at 16:13
  • OT: regarding: `board = (char**)malloc((*heightPtr) * sizeof(char*));` and `board[i] = (char*)malloc((*widthPtr) * sizeof(char));` 1) in C, the returned type is `void*` which can be assigned to any pointer. Casting just clutter the code and is error prone. Suggest removing the cast. 2) The expression: `sizeof( char )` is defined in the C standard as 1. Multiplying anything by 1 has no effect and just clutters the code. Suggest removing that expression. 3) always check (!=NULL) the returned value to assure the operation was successful. (cont) – user3629249 Apr 15 '20 at 16:17
  • (cont) if not successful, call `perror( "your error message");` to output both your error message and the text reason the system thinks the error occurred to `stderr` – user3629249 Apr 15 '20 at 16:18
  • regarding: `freeBoard(board, widthPtr);` Why 'free' the board memory, when you have just created it and have done nothing with it? – user3629249 Apr 15 '20 at 16:22
  • regarding: `void createBoard(int* widthPtr, int* heightPtr)` why pass pointers when the actual value/count is available? – user3629249 Apr 15 '20 at 16:24

1 Answers1

5
for (i = 0; i < *widthPtr; i++)
{
    board[i] = (char*)malloc((*widthPtr) * sizeof(char));
}

should be

for (i = 0; i < *heightPtr; i++)
{
    board[i] = (char*)malloc((*widthPtr) * sizeof(char));
}

Otherwise you iterate NCOLUMNS times (you want to iterate through the NROWS dimension)

Same for freeBoard()

Also, notice that in this line

board[i] = (char*)malloc((*widthPtr) * sizeof(char));

you prefer

board[i] = malloc(*widthPtr);

because:

1) There is no need to cast malloc and friends

2) sizeof(char) is guaranteed to be 1

David Ranieri
  • 39,972
  • 7
  • 52
  • 94