Question

The following code is an example from the NCURSES menu library. I'm not sure what could be wrong with the code, but valgrind reports some problems. Any ideas...

==4803== 1,049 (72 direct, 977 indirect) bytes in 1 blocks are definitely lost in loss record 25 of 36
==4803==    at 0x4C24477: calloc (vg_replace_malloc.c:418)
==4803==    by 0x400E93: main (in /home/gerardoj/a.out)
==4803== 
==4803== LEAK SUMMARY:
==4803==    definitely lost: 72 bytes in 1 blocks
==4803==    indirectly lost: 977 bytes in 10 blocks
==4803==      possibly lost: 0 bytes in 0 blocks
==4803==    still reachable: 64,942 bytes in 262 blocks

Source code:

#include <curses.h>
#include <menu.h>

#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
#define CTRLD   4

char *choices[] = {
    "Choice 1",
    "Choice 2",
    "Choice 3",
    "Choice 4",
    "Choice 5",
    "Choice 6",
    "Choice 7",
    "Exit",
}
;

int main()
{
    ITEM **my_items;
    int c;
    MENU *my_menu;
    int n_choices, i;
    ITEM *cur_item;

    /* Initialize curses */
    initscr();
    cbreak();
    noecho();
    keypad(stdscr, TRUE);

    /* Initialize items */
    n_choices = ARRAY_SIZE(choices);
    my_items = (ITEM **)calloc(n_choices + 1, sizeof(ITEM *));
    for (i = 0; i < n_choices; ++i) {
        my_items[i] = new_item(choices[i], choices[i]);
    }
    my_items[n_choices] = (ITEM *)NULL;

    my_menu = new_menu((ITEM **)my_items);

    /* Make the menu multi valued */
    menu_opts_off(my_menu, O_ONEVALUE);

    mvprintw(LINES - 3, 0, "Use <SPACE> to select or unselect an item.");
    mvprintw(LINES - 2, 0, "<ENTER> to see presently selected items(F1 to Exit)");
    post_menu(my_menu);
    refresh();

    while ((c = getch()) != KEY_F(1)) {
        switch (c) {
        case KEY_DOWN:
            menu_driver(my_menu, REQ_DOWN_ITEM);
            break;
        case KEY_UP:
            menu_driver(my_menu, REQ_UP_ITEM);
            break;
        case ' ':
            menu_driver(my_menu, REQ_TOGGLE_ITEM);
            break;
        case 10:
            {
                char temp[200];
                ITEM **items;

                items = menu_items(my_menu);
                temp[0] = '\0';
                for (i = 0; i < item_count(my_menu); ++i)
                if(item_value(items[i]) == TRUE) {
                    strcat(temp, item_name(items[i]));
                    strcat(temp, " ");
                }
                move(20, 0);
                clrtoeol();
                mvprintw(20, 0, temp);
                refresh();
            }
            break;
        }
    }
    unpost_menu(menu);
    free_item(my_items[0]);
    free_item(my_items[1]);
    free_item(my_items[2]);
    free_item(my_items[3]);
    free_item(my_items[4]);
    free_item(my_items[5]);
    free_item(my_items[6]);
    free_item(my_items[7]);
    free_menu(my_menu);
    endwin();
}
Was it helpful?

Solution

According to the NCURSES Programming Howto, using the menus library requires the following steps:

  • Initialize curses
  • Create items using new_item(). You can specify a name and description for the items.
  • Create the menu with new_menu() by specifying the items to be attached with.
  • Post the menu with menu_post() and refresh the screen.
  • Process the user requests with a loop and do necessary updates to menu with menu_driver.
  • Unpost the menu with menu_unpost()
  • Free the memory allocated to menu by free_menu()
  • Free the memory allocated to the items with free_item()
  • End curses
  • From what I can tell from your code:

    • You don't unpost the menu (which might cause a leak, or it might just risk garbling the screen).
    • The menu is freed after the items are freed (which I guess may or may not be a problem depending on how ncurses is implemented).
    • Only items 0 and 1 of the 8-element array of items are freed. This is probably a leak.
    • The my_items array of pointers is never freed. This is certainly a leak.

    As @lh3 said, compiling with the -g option will let Valgrind give the line number of lost memory.

    Edit (in response to your comment): my_items is a dynamically allocated array of pointers to dynamically created menu items. In other words, you have one block of dynamic memory, and it contains a bunch of pointers to a bunch of dynamically allocated ncurses structures (menu items). So, to clean up once you're done, you need to free each of the dynamically allocated ncurses structures, and then you need to free the block of memory that held the pointers to those structures.

    In other words, every calloc or malloc needs a free, every new_item needs a free_item, and so on.

    for (i = 0; i < n_choices; ++i) {
        free_item(my_items[i]);
    }
    free(my_items);
    

    OTHER TIPS

    Something to note with Valgrind (this comes up on the Valgrind user's mailing list often):

    still reachable: 64,942 bytes in 262 blocks
    

    This is just referencing blocks that were still reachable in main() at exit, which (under any modern kernel) would just be reclaimed by the OS anyway.

    While its good practice to explicitly free() every single allocated block before exit is called, this is not technically leaked memory, since it still could be reached at the time of exit.

    Focus on directly, indirectly and possibly lost blocks as Josh Kelly has suggested. This is just a supplement to the answers that have already pointed out the likely sources of leaks.

    try running valgrind with --leak-check=full maybe?

    free_item(my_items[7]);
    free(my_items);
    
    Licensed under: CC-BY-SA with attribution
    Not affiliated with StackOverflow
    scroll top