How can i find memory leaks in my program?

Question

Im building a terminal-based file viewer project with ncurses.h and C. Its only a side project as a hobby, and has a few lines of code. I get some segmentation errors and I don’t even have a clue where they are, if someone can check the code, I will appreciate a lot.

#include "window.h"
#include <dirent.h>
#include <menu.h>
#include <ncurses.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

static DIR *get_dir(const char *path)
{
    DIR *dir;
    if (strcmp(path, "current") == 0)
        dir = opendir(".");
    else
        dir = opendir(path);
    if (dir == NULL)
    {
        perror("Error opening dir");
        exit(-1);
    }
    return dir;
}

WINDOW *init_window(int start_y, int start_x, int y, int x)
{
    WINDOW *win;

    initscr();
    getmaxyx(stdscr, y, x);
    cbreak();
    noecho();

    if (x < WDIM_X || y < WDIM_Y)
    {
        start_x = (x - WDIM_X_MIN);
        start_y = (y - WDIM_Y_MIN);
        win = newwin(WDIM_Y_MIN, WDIM_X_MIN, 1, 2);
    }
    else {
        start_x = (x - WDIM_X) / 2;
        start_y = (y - WDIM_Y) / 2;
        win = newwin(WDIM_Y, WDIM_X, start_y, start_x);
    }

    box(win, 0, 0);
    start_color();
    curs_set(0);

    keypad(win, true);
    init_pair(1, COLOR_BLACK, COLOR_BLUE);
    wbkgd(win, COLOR_PAIR(1));
    wrefresh(win);
    return win;
}

void delete_window(WINDOW *win)
{
    delwin(win);
    endwin();
}

char *get_current_dir()
{
    /*
    char *dirs = malloc(MAX_LENGTH * sizeof(char));
    if (getcwd(dirs, sizeof(char) * MAX_LENGTH) != NULL)
        return dirs;
    return "Error";
    */
    char *cwd = getcwd(NULL, 0);
    return cwd;
}

char **dir_items(DIR *dir)
{

    char **current_dirs = (char **)malloc(MAX_LENGTH * sizeof(char *));
    struct dirent *entry;
    int i = 0;
    while ((entry = readdir(dir)) != NULL)
    {
        char *name = entry->d_name;
        int j=i;
        while (j > 0 && strcmp(name, current_dirs[j-1]) < 0)
        {
            current_dirs[j] = current_dirs[j-1];
            j--;
        }
        current_dirs[j] = name;
        i++;
    }
    return current_dirs;
}

void cd(const char *path)
{
    if (chdir(path) == -1)
    {
        perror("Error");
    }
}

void ls(const char *path)
{
    DIR *dir = get_dir(path);
    struct dirent *entry;
    while ((entry = readdir(dir)) != NULL)
        printf("%s | %s\n", path, entry->d_name);
    closedir(dir);
}

static int n_elements(char **items)
{
    int i = 0;
    while (items[i] != NULL)
    {
        i++;
    }
    return i;
}

/* static void moveMenu(MENU *menu, WINDOW *win)
{
    char c;
    WINDOW *w;
    while ((c = wgetch(win)) != '1')
    {
        switch (c)
        {
        case 'j':
            menu_driver(menu, REQ_DOWN_ITEM);
            wrefresh(win);
            break;
        case 'k':
            menu_driver(menu, REQ_UP_ITEM);
            wrefresh(win);
            break;
        case 'q':
            delete_window(win);
            exit(-1);
            break;
        case '\n':
            delete_window(win);
            win = init_window(WDIM_Y, WDIM_X, 5, 5);
            cd("/");
            sleep(3);
            render_dir(win);
            break;
        }
    }
} */

void render_dir(WINDOW *win)
{
    char **items_chars, *titulo, c;
    int i=0, x, y;
    int n;
    DIR *dir;
    ITEM **items;
    MENU *menu;
    char *path = get_current_dir();
    path = strcat(path, "/");
    dir = get_dir(path);
    items_chars = dir_items(dir);

    wclear(win);
    titulo = malloc(MAX_LENGTH * sizeof(char));
    strcpy(titulo, get_current_dir(path));
    mvwprintw(win, 0, 1, titulo);
    wrefresh(win);
    n = n_elements(items_chars);
    items = (ITEM **)calloc(n + 1, sizeof(ITEM *));

    for (int j = 0; j < n; j++)
    {
        items[j] = new_item(items_chars[j], "");
    }
    // final have to be NULL
    items[n] = (ITEM *)NULL;

    menu = new_menu(items);
    set_menu_win(menu, win);
    set_menu_sub(menu, derwin(win, 25, 40, 2, 1));
    // set_menu_fore(menu, A_NORMAL);
    set_menu_format(menu, 25, 1);
    set_menu_back(menu, COLOR_PAIR(1));
    post_menu(menu);
    wrefresh(win);
    while ((c = wgetch(win)) != '1')
    {
        switch (c)
        {
        case 'j':
            menu_driver(menu, REQ_DOWN_ITEM);
            if (i > n)
                i = n;
            else
                i++;
            //i++;
            wrefresh(win);
            break;
        case 'k':
            menu_driver(menu, REQ_UP_ITEM);
            if (i < 0)
                i = 0;
            else
                i--;
            //i--;
            wrefresh(win);
            break;
            break;
        case 'q':
            delete_window(win);
            exit(-1);
            break;
        case '\n':
            for (int j = 0; j < n; j++)
            {
                free_item(items[j]);
            }
            free_menu(menu);
            path = strcat(path, items_chars[i]);
            free(items_chars);
            free(titulo);
            closedir(dir);
            wclear(win);
            delwin(win);
            getmaxyx(stdscr, y, x);
            if (x < WDIM_X || y < WDIM_Y)
                win = init_window(WDIM_Y_MIN, WDIM_X_MIN, 5, 5);
            else
                win = init_window(WDIM_Y, WDIM_X, 5, 5);
            cd(path);
            free(path);
            mvwprintw(win, 20, 1, "s");
            render_dir(win);
            break;
        }
    }

    wrefresh(win);
}

Here is main

#include "window.h"

int main(int argc, char **argv)
{
    WINDOW *win;
    win = init_window(WDIM_Y, WDIM_X, 5, 5);
    render_dir(win);
    delete_window(win);
    return 0;
}

I have tried to free all the memory in all ways but it still has segmentation faults and memory leaks.

All you're going to hear is [Valgrind](https://valgrind.org). Note that segmentation faults are not caused by memory leaks, but by invalid access. A "use after free" bug one way to do this. — tadman, May 18 '23 at 21:59
You need to post your code here as text, ideally as a [mre]. If you just have a link to some other site, your question will be closed and deleted. — pmacfarlane, May 18 '23 at 22:01
In particular, a link to github is useless here. Surely you're going to fix the code there, so the bug will be gone when people read this question in the future. — Barmar, May 18 '23 at 22:02
You might also consider running your program under control of a debugger (though that might be tricky for a curses application). That would enable you to pinpoint exactly where each segfault occurs, to and look at the values of relevant variables to try to determine what went wrong. — John Bollinger, May 18 '23 at 22:04
@JohnBollinger I believe gdb can connect to a running process - e.g. in a different terminal. Might be the way to go. — pmacfarlane, May 18 '23 at 22:07
This looks very dodgy: `path = strcat(path, "/");` Is there space in that buffer for another character? — pmacfarlane, May 18 '23 at 22:09
`getcwd()` (as you call it) returns a buffer the size of the string it returns. There is no space to add a character. Read the [man page for getcwd()](https://man7.org/linux/man-pages/man3/getcwd.3.html) to see why. — pmacfarlane, May 18 '23 at 22:25
@Apesteguia You're not supposed to go and edit your question at the end and make all these comments look irrelevant. You could post an answer to your own question explaining how it got fixed. Or wait for someone to do so. — pmacfarlane, May 18 '23 at 22:40
*"All you're going to hear is Valgrind."* Alternatively, the address sanitizer (and UB sanitizer). — HolyBlackCat, May 18 '23 at 22:48
@jxh why? I have looked in several books and tutorials and this type of funciton shows all the time to init the window `WINDOW *create_newwin(int height, int width, int starty, int startx) { WINDOW *local_win; local_win = newwin(height, width, starty, startx); box(local_win, 0, 0); wrefresh(local_win); return local_win; } ` — Apesteguia, May 18 '23 at 22:51
The `win` parameter is pass by value. That means the `win` parameter is just a local variable that gets its initial value from the function caller. Assigning a value to a local variable does not allow that value to get returned to the caller. — jxh, May 18 '23 at 23:01
The leak occurs assuming there is a way to get `wgetch()` to return something other than `1`. — jxh, May 18 '23 at 23:04

pmacfarlane · Accepted Answer · 2023-05-18T23:05:31.537

The problem (or a problem, there may be others) is that you are doing this:

char *path = get_current_dir();
path = strcat(path, "/");

get_current_dir() does this:

char *cwd = getcwd(NULL, 0);
return cwd;

And getcwd() does this, according to the man page

As an extension to the POSIX.1-2001 standard, glibc's getcwd() allocates the buffer dynamically using malloc(3) if buf is NULL. In this case, the allocated buffer has the length size unless size is zero, when buf is allocated as big as necessary. The caller should free(3) the returned buffer.

So the buffer returned is exactly the size of the path. There is no space to add a '/'.

So this:

path = strcat(path, "/");

is causing a buffer overflow, and undefined behaviour.

How can i find memory leaks in my program?

1 Answers1