9

I am trying to store list of files in a char** variable.

scandir() finishes properly but I get a segmentation fault when trying to print the char**.

Here's the code:

int main()
{
    char** fileList;
    int noOfFiles;
    char* path = ".";
    makeList(&fileList, &noOfFiles, path); 
    return 0;
}

void makeList(char ***fileList, int* noOfFiles, char* path){
    struct dirent **fileListTemp;
    *noOfFiles = scandir(path, &fileListTemp, NULL, alphasort);
    int i;
    fileList = (char***)malloc(sizeof(char***));
    *fileList = (char**)malloc(*noOfFiles * sizeof(char*));
    printf("total: %d files\n",*noOfFiles);
    for(i = 0; i < *noOfFiles; i++){
        *fileList[i] = (char*)malloc(strlen(fileListTemp[i] -> d_name) *sizeof(char));
        strcpy(*fileList[i], fileListTemp[i] -> d_name);
        printf("%s\n",*fileList[i]);
    }
    return;
}

This gives a segmentation fault after printing 2 file names.

output:

total: 27 files.

..

.j.v

Segmentation fault (core dumped)

Community
  • 1
  • 1
kBisla
  • 550
  • 2
  • 6
  • 22
  • 2
    [Please don't cast malloc() in C.](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – chrylis -cautiouslyoptimistic- Aug 23 '13 at 12:08
  • 1
    no need to cast the memory allocated? Elaborate please. – kBisla Aug 23 '13 at 12:09
  • 2
    There's a very good explanation at that link. Regarding your question, you're passing in `char ***fileList` (a pointer to the declared variable, which is `char **fileList` in `main`), but then writing directly to the value on the stack. Did you mean to say `*fileList = malloc(sizeof(char**))`? Using the same name for differing levels of pointers gets confusing fast. – chrylis -cautiouslyoptimistic- Aug 23 '13 at 12:13
  • `*fileList = malloc(sizeof(char**))` actually yes. But don't I need to specify the no. of string this is going to contain? I understand that deep down every pointer just stores a memory location but no of strings has to be stored somewhere...? – kBisla Aug 23 '13 at 12:33
  • Do you have some time to work on this project, or is it time-sensitive? I'll help you in chat, but I don't have much brainpower left right at the moment. – chrylis -cautiouslyoptimistic- Aug 23 '13 at 12:37
  • Ive about 8 hrs left before I've to submit this assignment. Where do I chat? – kBisla Aug 23 '13 at 12:43
  • 1
    I won't be able to help you before your assignment's due, sorry. I have a few suggestions: (1) Use distinct names for "levels" of pointers (don't declare `int *i` and then have `int **i` in a signature); (2) consider a temp variable in `makeList` to hold the newly allocated pointer for clarity; (3) sketch out on paper with boxes and arrows where each of your assignments should be going to ensure you're indirecting correctly; (4) use simplifying C conventions like not casting `malloc` and never saying `sizeof(char)` (always 1 by definition). – chrylis -cautiouslyoptimistic- Aug 23 '13 at 12:51

4 Answers4

22

The function scandir() allocates the memory for you.

You do not need to allocate ANY memory. You DO need to free the memory returned to you by scandir().

Your code calls: *noOfFiles = scandir(path, &fileListTemp, NULL, alphasort);

On return, noOfFiles will contain the number of directory entries in the path directory, and fileListTemp will point to an allocated array of pointers to allocated struct dirent blobs each of which has a d_name member which points to the null-terminated name of a file/directory.

If your directory contains the files "FirstFile.txt", "AnotherFile.txt", "ThirdFile.txt", for example, with your call, upon return from scandir(), noOfFiles will be set to 5 for the three files plus two more for the "." and ".." directory entries. THE ENTRIES WILL BE IN NO PARTICULAR ORDER IF YOU DO NOT PASS 'alphasort'. (Actually that's a little incorrect. They will be in the order of the directory filename entries which depends on the order in which the files were originally created.)

Because you passed 'alphasort' you should see the entries in the following order (I am explicitly showing the null-byte-string-terminator:

fileListTemp[0]->d_name == ".\0"
fileListTemp[1]->d_name == "..\0"
fileListTemp[2]->d_name == "AnotherFile.txt\0"
fileListTemp[3]->d_name == "FirstFile.txt\0"
fileListTemp[4]->d_name == "ThirdFile.txt\0"

So fileListTemp points to a block of allocated memory holding five struct dirent pointers. Each of the five struct dirent pointers points to a struct dirent block of allocated memory containing a null-terminated directory entry name in the d_name member. (Even this is a simplification, because the d_name entry is also a pointer, but it points to extra allocated space at the tail end of the allocated block, and the entry name is stored there.)

That is SIX blocks of allocated memory.

You can use this allocated memory until you are done with it, and then you call free() on EACH entry in the array followed by free() of the array itself.

You MUST free every entry as well as the array itself. They are all independently allocated blobs of memory.

When you are done with the list you should:

for (int i = 0; i < noOfFiles; i++)
  {
  free(fileListTemp[i];
  }

free(fileListTemp);
7

This is an old question, but since I came upon it and it did not solve my question as effectively as the man page did, I am copying a code snippet from the man page as a new answer for the future.

  #include <dirent.h>

   int
   main(void)
   {
       struct dirent **namelist;
       int n;

       n = scandir(".", &namelist, NULL, alphasort);
       if (n < 0)
           perror("scandir");
       else {
           while (n--) {
               printf("%s\n", namelist[n]->d_name);
               free(namelist[n]);
           }
           free(namelist);
       }
   }
merlin2011
  • 63,368
  • 37
  • 161
  • 279
1

modify the code, it works!

  #include <stdio.h>
  #include <sys/types.h>
  #include <sys/dir.h>
  #include <stdlib.h>
  #include <string.h>

  void makeList(char ***fileList, int* noOfFiles, char* path){
      struct dirent **fileListTemp;
      *noOfFiles = scandir(path, &fileListTemp, NULL, alphasort);
      int i;
      *fileList = (char**)malloc(*noOfFiles * sizeof(char*));
      printf("total: %d files\n",*noOfFiles);
      for(i = 0; i < *noOfFiles; i++){
          (*fileList)[i] = (char*)malloc(strlen(fileListTemp[i] -> d_name)+1);
          strcpy((*fileList)[i], fileListTemp[i] -> d_name);
          printf("%s\n",(*fileList)[i]);
      }   
      return;
  }

  int main()
  {
      char** fileList;
      int noOfFiles;
      char* path = ".";
      makeList(&fileList, &noOfFiles, path); 
      return 0;
  }

fileList is type of char ***, so *fileList is the fileList variable in main function.

*fileList = (char**)malloc(*noOfFiles * sizeof(char*));

with this statement, *fileList points to allocated memory of array of pointers.
if want to allocate memory for each pointer within the array, we need use (*fileList)[i], other than *fileList[i], the precedence of [] is higher than *.

Hui Lee
  • 11
  • 3
  • Your construction of fileList is correct, and properly extracts the strings from the dirent objects, unfortunately it leaks memory because the blobs of memory pointed to by fileListTemp are never freed. If makeList() is called a ton of times, e.g. by a function similar to file-tree-walker -- https://linux.die.net/man/3/nftw -- each call will leak memory from the heap. Yes, the memory is recovered when the app exits in modern OSes, but it will still crash the app if called enough times, and with embedded systems your app typically only "exits" on power-cycle. – kch_PE_MSEE_BSCE Mar 31 '20 at 01:26
0

I am not sure this is the problem, but you have to allocate another byte for the null termination:

*fileList[i] = (char*)malloc(strlen(fileListTemp[i] -> d_name + 1) *sizeof(char));
eyalm
  • 3,258
  • 17
  • 21