C: Why freeing int** array isn't complete?

Question

I allocate memory for my 2d array, it works... but not correctly. When I use valgrind to see if I allocate and free well my array, I see that I free only 29 blocs on 445. The other problem is that my array got 11 columns for 20 lines, so 220 blocs I think. My results aren't correct, isn't it? How can I fix it?

There is my code, thank you in advance for your answers.

EDIT: My program should work with ncurses library. Should I really remove all what is link to?

There is all my code this time:

#include <ncurses.h>
#include <stdlib.h>
#include <unistd.h>

int main(void) {
    initscr();
    noecho();
    curs_set(0);
    keypad(stdscr, TRUE);
    if (init_board() == -1)
        return (-1);
    while (1)
        refresh();
    endwin();
    return (0);
}

void print_board(int **tab) {
    int i;
    int line;

    i = 0;
    printw("------------\n");
    while (i < 20) {
        line = 11;
        printw("| ");
        while (line-- > 0) {
            tab[i][line] = 0;
            if (tab[i][line] == 0)
                printw("* ");
        }
        printw("|\n");
        i++;
    }
    wprintw(stdscr, "------------\n");
}

int **board_size(int **tab) {
    int i;

    i = 0;
    while (i < 20) {
        if ((tab[i] = (int *)malloc(sizeof(int) * 11)) == NULL) {
            wprintw(stdscr, "%s\n", "Second malloc's tab failed.");
            return (NULL);
        }
        i++;
    }
    return (tab);
}

void free_board(int **tab) {
    int   i;

    i = 0;
    while (i < 20) {
        printw("%d\n", i);
        free(tab[i]);
        i++;
    }
    free(tab);
}

int init_board() {
    int **tab;

    tab = NULL;
    if ((tab = (int **)malloc(sizeof(int*) * 20)) == NULL) {
        wprintw(stdscr, "%s\n", "First malloc's tab failed.");
         return (-1);
    }
    if ((tab = board_size(tab)) == NULL)
        return (-1);
    print_board(tab);
    free_board(tab);
    return (0);
}

I don't see any *obvious* mistakes. Please post a *complete program* that we can compile for ourselves. Please also remove all uses of the nonstandard `printw`, `wprintw`, `stdscr`, etc. as they distract from the question. — zwol, Mar 08 '16 at 23:03
`int **` is not a 2D array, but a "pointer to pointer to int". It cannot even point to a 2D array. And don't cast the result of `malloc` & friends in C. If you want a 2D array, why not use one? — too honest for this site, Mar 08 '16 at 23:07
There is no need to return an `int**` from `board_size` as `board_size` manipulates `tab` directly. — callyalater, Mar 08 '16 at 23:13
I edited my post to add more precision about my code. But I think I understand the problem, I didn't think I didn't use a 2d array... I have a lot of things to learn about C (: — devill_h, Mar 08 '16 at 23:16
@Olaf This is the "dope vector" idiom. The resulting group of objects can be used as a 2D array, and AFAIK you _have_ to do something like this when both dimensions of the array are determined at runtime; the 2D arrays you're thinking of can only have _one_ runtime-variable dimension. — zwol, Mar 09 '16 at 11:43
@Eino You needn't edit out the ncurses stuff. I wouldn't have said that if you had posted a complete program in the first place, because then I would have known it was ncurses stuff. — zwol, Mar 09 '16 at 11:45
@zwol: It cannot be used as 2D array. It just has the same syntax, but very different implications, e.g. for allocation/free (which is a recurring problem here) and for performance (fragmentation, locality, alignment), depending on the algorithm. And you can very well use a VLA with multiple dimensions fixed. But for a 2D array, a simple `int (*)[inner_dim]` is sufficient ("pointer to 1D VLA"). I elaborated on that in an [answer](http://stackoverflow.com/a/35615201/4774918) — too honest for this site, Mar 09 '16 at 13:22
@Olaf VLAs are still very poorly supported and IMNSHO should not be used in portable code. It's possible to avoid most of the problems you're thinking of by allocating the storage for the pseudo-2D array in one big block and then carving it up. And the statement "It cannot be used as a 2D array" is technically true but will lead someone at the OP's level of understanding toward further confusion, not toward enlightenment. — zwol, Mar 09 '16 at 15:21
@zwol: VLAs are part of the C standard. C99 is now 17 years old, and any modern compiler supports it (actually it should be C11, but the commitee made the stupid decission to make them optional; anyway most compilers support both versions if any). If not use a modern compiler. Telling a beginner this is not an 2D array and does not have the same implications is not confusing. On the contrary, stating that this is a 2D array as done here by people who should know better actually is. Only if one understands the implications, it is possible to decide which approach to use. — too honest for this site, Mar 09 '16 at 15:32
@Olaf The fact that C11 made this feature of C99 optional should tell you something about how unenthusiastic compiler vendors have been about picking it up. Re pedagogy, you told a beginner that something indexable as `[x][y]` "isn't a 2D array," without any further explanation. How can this possibly _not_ lead to confusion? — zwol, Mar 09 '16 at 15:40
@zwol: Not really. There is only one compiler vendor who likely initiated that breaking with the commitees normal approach of keeping older features, however obsolete they have become. About the didactics: This is not school, nor a tutorial site. A poster can very well be expected to do learn on his own. The difference is emphasised in every good C book. Also there are enough posts on SO about this subject already. Yesterday e.g. there were at least 3 new posts. — too honest for this site, Mar 09 '16 at 15:45
@Olaf You are wrong on all counts and I am done arguing with you. — zwol, Mar 09 '16 at 16:23

score 2 · Accepted Answer · edited May 23 '17 at 12:19

There is nothing wrong with your code, and you are freeing all memory you allocate. What you do not, and cannot control is the memory allocated by the curses library for managing the various windows and curses environment. For example:

valgrind Output

$ valgrind ./bin/board
==18521== Memcheck, a memory error detector
==18521== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==18521== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==18521== Command: ./bin/board
==18521==
==18521==
==18521== HEAP SUMMARY:
==18521==     in use at exit: 199,908 bytes in 440 blocks
==18521==   total heap usage: 468 allocs, 28 frees, 205,540 bytes allocated
==18521==
==18521== LEAK SUMMARY:
==18521==    definitely lost: 0 bytes in 0 blocks
==18521==    indirectly lost: 0 bytes in 0 blocks
==18521==      possibly lost: 0 bytes in 0 blocks
==18521==    still reachable: 199,908 bytes in 440 blocks
==18521==         suppressed: 0 bytes in 0 blocks
==18521== Rerun with --leak-check=full to see details of leaked memory
==18521==
==18521== For counts of detected and suppressed errors, rerun with: -v
==18521== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 1 from 1)

You are allocating 1040 bytes (on x86_64) (960 bytes on x86) for your 220 integers and 20 pointers. You have no control over the remaining ~198,000 bytes allocated by routines in the curses library.

So you are not going crazy and you are freeing what you are supposed to. When you program with a library that allocates blocks of memory, you should check if there is a valgrind pattern or exclusion file that will exclude the curses allocation (much like is done for GTK/glib) Windowed environments pre-allocate large blocks to handle the various tasks they preform. That memory will always be shown as still in use when your code exits. (the library may or may not tidy up before the ultimate exit of the program).

However, as others have pointed out, you should not cast the return of malloc. See: Do I cast the result of malloc? for thorough explanation. Your allocations should more properly be, e.g.:

if ((tab = malloc (sizeof *tab * 20)) == NULL) {
...
    if ((tab[i] = malloc (sizeof **tab * 11)) == NULL) {

score 0 · Answer 2 · edited May 23 '17 at 12:06

0

This answer is not correct. I did't realize that pointers array is allocated in one routine and integers arrays are allocated in other routine.

You do malloc in board_size and in initialize_board. Former doesn't allocate array of pointers. Looks strange.

As was mentioned, try to remove everything unrelated first. It's possible that some libraries allocate memory on initialization and don't release it until program terminates.

By the way, it's easier and more effective to allocate n*m once array than allocate n+1 arrays.

edited May 23 '17 at 12:06

Community

1
1

answered Mar 08 '16 at 23:07

George Sovetov

4,942
5
36
57

Can you explain me more clearly what do you mean by "allocate n*m once array than allocate n+1 arrays" please? I'm not sure to understand :s – devill_h Mar 08 '16 at 23:23
@Eino allocate once array of 220 integers and access as `a[row*20+column]`. – George Sovetov Mar 08 '16 at 23:28
Why two **free()** statements ? – Arif Burhan Mar 08 '16 at 23:29
And two **malloc ()** statements, which don't pair with the **free()** statements. – Arif Burhan Mar 08 '16 at 23:32
@George Thank you, I'll try it! – devill_h Mar 08 '16 at 23:34
@Eino glad it helped you. – George Sovetov Mar 08 '16 at 23:35
@ArifBurhan I use free first time for all the lines I allocated, in a loop, then for the column I allocated at beginning. – devill_h Mar 08 '16 at 23:35

C: Why freeing int** array isn't complete?

2 Answers2