Adding to strings in C while listing Directories

Question

I am trying to list a directories in a specific Dir in C. But the problem is when I want to add the path and the dir name. I wrote a function to add the path and dir name.

void setNewString(char* str1,char* str2)
{
    // Declare new buffer
    char* buffer = malloc(strlen(str1) + 1);

    // Set the buffer to be str1
    int m = 0,n = 0, o = 0;
    for (int p = strlen(str1); m < p; m++)
    {
        buffer[m] = str1[m];
    }
    buffer[m] = '\0';

    // Set str1 to be str2
    for(int q = strlen(str2);n < q; n++)
    {
        str1[n] = str2[n];   
    }

    // Continue the str1 and buffer to end.
    while(buffer[o] != '\0')
    {
        str1[n] = buffer[o];
        n++;
        o++;
    }
    str1[n] = '\0';

    // Free Buffer
    free(buffer);   
}

And there is while loop to add the dir name together

while ((awesome_controller = readdir(awesome_dir)) != NULL)
{
    if ((awesome_controller->d_type) == 4 && strcmp(awesome_controller->d_name,"..") != 0 && strcmp(awesome_controller->d_name,".") != 0)
    {
      // List All the folders.
      size = ((strlen(temp_awesome)) + (strlen(awesome_controller->d_name)) + 2);
      baby_dir[i] = malloc(size);
      printf("Size:%i\n",size);
      baby_dir[i] = awesome_controller->d_name;
      setNewString(baby_dir[i],temp_awesome);
      i++;  
    }
}

for example if I type in the dir: home/khaledmohammad/vhosts/

I get only 1 dir path: Output:

Size:30
Dir 0: /home/khaledmohammad/vhosts/vcipher

But I know there are 3 dirs.

But when I change the setNewString() function's 15th number line

To

for(int q = strlen(str1);n < q; n++)

From

for(int q = strlen(str2);n < q; n++)

it seems to give me 3 dirs output but not complete.

Output:

Size:30
Size:28
Size:32
Dir 0: /home/kvcipher
Dir 1: /homepset7
Dir 2: /home/khalocalhost

This happens because obviously when is set it to strlen of str2 the loop exits mysteriously.And I don't know why.

So my questions are:

1) Why this happens?

2) How to fix it?

3) And was my setNewString() function good enough or I have mistakes or there is a better way of doing it?

`baby_dir[i] = awesome_controller->d_name;` --> `strcpy(baby_dir[i], awesome_controller->d_name);` — BLUEPIXY, Oct 21 '14 at 19:04
You should introduce yourself to the `strcpy()` function. It would like to be your friend. You might also like `strdup()` and `strcat()`. All of these have a bit of a reputation, however, since (like your code) they are prone to buffer overruns and/or memory management surprises. — John Bollinger, Oct 21 '14 at 19:05
Yes, you overwrote the memory allocation pointer with another pointer. The whole thing looks very convoluted. You've copied `str1` to `buffer` and then `str2` to `str1` and finally appended `buffer`. Why didn't you just pass the arguments the other way round and append one to the other? Finally the way you organise your `for ()` statements is rather peculiar. — Weather Vane, Oct 21 '14 at 19:16

score 0 · Accepted Answer · answered Oct 21 '14 at 19:19

Here is setNewString() implemented via the standard string manipulation functions:

void setNewString(char* str1,char* str2)
{
    // Create a copy of str1 in newly-allocated memory
    char* buffer = strdup(str1);

    // make the existing str1 a copy of str2
    strcpy(str1, str2);    

    // append buffer to str1
    strcat(str1, buffer);

    // free the buffer
    free(buffer);   
}

Note also that whereas this...

baby_dir[i] = malloc(size);

... is basically ok, when you combine it with this ...

baby_dir[i] = awesome_controller->d_name;

... you have a memory leak. It looks like you probably want the second line to be a strcpy() instead, as @BLUEPIXY suggested.

But then you're seriously overlapping the actual behavior of setNewString(). Personally, I'd drop setnewString() altogether, and just write the main loop like this:

while ((awesome_controller = readdir(awesome_dir)) != NULL)
{
    if ((awesome_controller->d_type) == 4 && strcmp(awesome_controller->d_name,"..") != 0 && strcmp(awesome_controller->d_name,".") != 0)
    {
      // List All the folders.
      size = ((strlen(temp_awesome)) + (strlen(awesome_controller->d_name)) + 2);
      baby_dir[i] = malloc(size);
      printf("Size:%i\n",size);
      strcpy(baby_dir[i], temp_awesome);
      strcat(baby_dir[i], awesome_controller->d_name);
      i++;  
    }
}

Thankyou Very Much! But I have a question about my setNewString() function! What was really wrong about it? — Khaled Mohammad, Oct 22 '14 at 07:23
Other than the fact that the original `setNewString()` spent (conservatively) 13 complicated lines of code to perform a job that could have been done in four simple lines, I'm uncertain that there was anything actually *wrong* with it. It does make a potentially assumption about the capacity of its `str1` argument, but the simpler version of that function makes the same assumption. The real problem in your code was here: `baby_dir[i] = awesome_controller->d_name;`. Not only do you have a memory leak, as I described already, but you may not get the string capacity you need. — John Bollinger, Oct 22 '14 at 13:27
Also, assigning the string instead of copying it may create an aliasing problem. (Meaning, in this case, that different elements of `baby_dir` and member `awesome_controller->d_name` all end up being pointers to the **same string**.) — John Bollinger, Oct 22 '14 at 13:29
Oops, "potentially assumption" -> "potentially *unsafe* assumption". Too late to edit. — John Bollinger, Oct 22 '14 at 13:34

Adding to strings in C while listing Directories

1 Answers1