1

I am using a tutorial to make a piece of code that reads a directory and dynamically populates an array with the files names.

Firstly, the code below does not work, because it populates the entire array with just one file name - the very last file name in the directory. Secondly, I am having trouble understanding some pieces of it. After the code, I am including a list of questions. 

char** directory_string_array(char* directory){
    DIR *dp;
    struct dirent *ep;
    dp = opendir(directory);
    char* current_directory;

    char **string_array = NULL;
    int i = 0, strcount = 0;


    if (dp !=NULL){
        while (ep = readdir (dp)){
            //pointer to char array which contains name of current file
            current_directory = ep->d_name;
            //allocate additional memory to string_array
            string_array = (char**) realloc(string_array, (strcount+1) * sizeof(char*));
            string_array[strcount++] = current_directory;
        }
        (void) closedir(dp);

    } else{
        perror("Couldn't open the directory");
    }
    //print the array to check it
    for(i = 0; i <strcount; i++){
        printf("strarray[%d] == %s\n", i, string_array[i]);
    }
    //free memory (this will later be outsourced to another file - I know that this will free memory from the thing I am trying to return
    for(i = 0; i < strcount; i++){
        free(string_array[i]);
    }
    free(string_array);

    return string_array;
}

The resulting array should be:

Array = {".", "..", "File1", "File2"}

But instead it is:

Array = {"File2", "File2", "File2", "File2"}

Question 1:

char **string_array = NULL;

I noticed that this "pointer to pointer" concept is used a lot, and I think I am misunderstanding why/how it is used. I thought that a pointer to anything is just a pointer (a memory chunk which holds an address to something). Why do we care that this particular pointer points to another pointer? Is this just notation or does the compiler treat ** different from *?

Question 2:

string_array = (char**) realloc(string_array, (strcount+1) * sizeof(char*));

So here we allocate an additional block of memory of same size as char* (I take it that it is of the same size as char**). However, why do we not allocate memory for individual members of the string array? So why do we not use this line of code next, for example:

string_array[0] = malloc(sizeof(char*));
Improve this question
2
  • The answer to your question #2 is "because it's a bug in code". You should do the malloc for the individual string elements as you go through results of readdir. You should also assign results of realloc to a temporary, not to the string_array, to avoid writing over a NULL on out-of-memory errors. Commented Jul 9, 2015 at 17:26
  • Standard warning: Do not cast void * as returned by malloc & friends! Commented Jul 9, 2015 at 17:50

3 Answers 3

Reset to default
1

The first problem is that your code does not duplicate ep->d_name returned by readdir. That is why your code print the last returned item.

You can fix it by calling strdup on current_directory, or by duplicating strings yourself:

size_t len = strlen(current_directory)+1;
string_array[strcount] = malloc(len);
strcpy(string_array[strcount], current_directory);
strcount++;

You should also split your function in two - one creating the array, and one freeing it. This way you would avoid undefined behavior from accessing freed memory:

char** directory_string_array(char* directory, size_t *strcount){
    ...
    //print the array to check it
    for(i = 0; i < *strcount; i++){
        printf("strarray[%d] == %s\n", i, string_array[i]);
    }
    return string_array;
}
void free_directory_string_array(char **string_array, size_t strcount) {
    for(i = 0; i < strcount; i++){
        free(string_array[i]);
    }
    free(string_array);
}
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1 
string_array[strcount++] = current_directory;

You're setting every element in string_array to point to current_directory, which points to the string "File2" when you exit the loop. Your second question -- "why do we not allocate memory for individual members of the string array?" -- is exactly on point. Maybe you want something like:

string_array[strcount++] = strdup(current_directory);
Improve this answer

Comments

1

If you read the readdir(3) manual page you will see:

The data returned by readdir() may be overwritten by subsequent calls to readdir() for the same directory stream.

and

On success, readdir() returns a pointer to a dirent structure. (This structure may be statically allocated...

[Emphasis mine]

This means that the pointer you get might be the same no matter how many times you call readdir. That also means that the pointer you store will be the same every time.

You need to copy the actual string instead of just the pointer. This can be done by e.g. strdup.

Improve this answer

2 Comments

I know that this is incorrect for now. However, I just wanted to see if the string array would print correctly first. This free part will be outsourced to another function later. However, the freeing happens after the print - not before, and it is the print that is messing up. "for(i = 0; i <strcount; i++){ printf("strarray[%d] == %s\n", i, string_array[i]); }" This is what gives the wrong answer.
@RebeccaK375 Ah, now I know what's wrong, I'll edit my answer accordingly.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.