char** allocation and how to store values

Question

    void allocateFolderTree(char **tree)
    {
        int i;
        tree = (char **)malloc(sizeof(char*)*MAX_FOLDERS);
        for(i=0;i<MAX_FOLDERS;i++)
          tree[i] = (char *)malloc(sizeof(char)*MAX_FOLDERS*MAX_FILENAME);    
    }

    void getFolderTree (char **tree, char *path, int i)
    {
        DIR *dir = opendir(path);
        struct dirent *entry;
        while (entry = readdir(dir)) 
        {
        if( !strcmp(entry->d_name, ".") || !strcmp(entry->d_name, ".."))
            continue;   

        if (entry->d_type & DT_DIR)//check file type
        {
            //segfault for the next 3 lines
            strcpy(tree[i], path);
            strcat(tree[i], "/");
            strcat(tree[i], entry->d_name);

            i++;

            char *new_path = malloc(sizeof(char)*(strlen(path)+MAX_FILENAME));
            strcpy(new_path, path);
            strcat(new_path, "/");
            strcat(new_path, entry->d_name);
            getFolderTree(tree, new_path, i);
            free(new_path);
        }
        }
        closedir (dir);
    }

int main ()
{
    char **folderTree;
    allocateFolderTree(folderTree);
    getFolderTree(folderTree, ROOT, 0);

    free(folderTree);
    return 0;
}

Why am I getting segfault and how do I solve this?

PS: MAX_FOLDERS=1000 MAX_FILENAME=30 The folders I'm reading are less than 10 and each has a name less than 30!

Answer 1

This fixes your memory allocation problems.

I changed a little. Your main problem was the first malloc in allocateFolderTree. In allocateFolderTree you can dereference to one level and malloc, but not up two levels which over-writes the address of folderTree (the argument) and that means when the function returns you have no way of accessing folderTree.

MAX_FOLDERS is baaaad. You need to calculate the number of folders first and then allocate based on this number.

Eg when I run the program, folderTree has an address of: 0x0021edf0.

Each folderTree array element is a null pointer before allocateFolderTree is called.

Then in allocateFolderTree each array element is allocated memory on the heap.

When you return from allocateFolderTree, the tree parameter still retains the address 0x0021edf0 but each array element now contains the address of memory on the heap.

#define MAX_FOLDERS 1000
#define MAX_FILENAME 256

#include <sys/types.h>
#include "dirent.h"

void allocateFolderTree(char** tree)
{
   int i;
   for(i=0;i<MAX_FOLDERS;i++)
      tree[i] = (char *)malloc(sizeof(char)*MAX_FOLDERS*MAX_FILENAME);    
}

void getFolderTree (char **tree, char *path, int i)
{
   DIR *dir = opendir(path);
   struct dirent *entry;
   while (entry = readdir(dir)) 
   {
      if( !strcmp(entry->d_name, ".") || !strcmp(entry->d_name, ".."))
         continue;   

      if (entry->d_type & DT_DIR)//check file type
      {
         //segfault for the next 3 lines
         strcpy(tree[i], path);
         strcat(tree[i], "/");
         strcat(tree[i], entry->d_name);

         i++;

         char *new_path = (char*)malloc(sizeof(char)*(strlen(path)+MAX_FILENAME));
         strcpy(new_path, path);
         strcat(new_path, "/");
         strcat(new_path, entry->d_name);
         getFolderTree(tree, new_path, i);
         free(new_path);
      }
   }
   closedir (dir);
}

int main ()
{
   char* ROOT = "/";
   char* folderTree[MAX_FOLDERS] = {0};
   allocateFolderTree(folderTree);
   getFolderTree(folderTree, ROOT, 0);

   //You will have to create your own free_foldertree function to free each array item
   //free(folderTree);
   return 0;
}

Answer 2

This code:

void allocateFolderTree(char **tree)
{
    int i;
    tree = (char **)malloc(sizeof(char*)*MAX_FOLDERS);
    for(i=0;i<MAX_FOLDERS;i++)
      tree[i] = (char *)malloc(sizeof(char)*MAX_FOLDERS*MAX_FILENAME);    
}

modifies the local copy of tree in the function, but never returns it to the calling code, so the allocated memory is all lost immediately the function returns. You have at least two choices on how to fix this:

char **allocateFolderTree(void)
{
    int i;
    char **tree = (char **)malloc(sizeof(char*)*MAX_FOLDERS);
    for(i=0;i<MAX_FOLDERS;i++)
      tree[i] = (char *)malloc(sizeof(char)*MAX_FOLDERS*MAX_FILENAME);
    return tree;  
}

or:

void allocateFolderTree(char ***tree)
{
    int i;
    *tree = (char **)malloc(sizeof(char*)*MAX_FOLDERS);
    for(i=0;i<MAX_FOLDERS;i++)
      (*tree)[i] = (char *)malloc(sizeof(char)*MAX_FOLDERS*MAX_FILENAME);    
}

On the whole, avoiding triple pointers is a good idea, so I'd normally go with the other option.

I also observe that all the loops are allocating a lot of space in the inner loop. Are you sure you want to include MAX_FOLDERS in the size? On the face of it, you should be allocating the size of MAX_FILENAME, not MAX_FOLDERS * MAX_FILENAME. (Each inner allocation is currently allocating around 30 KiB, so in total you're allocating around 30 MiB of space.)

Note that sizeof(char) == 1 by definition, so there is little need to include it in the size calculation.