Why does opendir() change the file path string passed to it?

Question

I am trying to write a function given a current working directory will print all the contents of that directory as well as the contents in the sub directories in the cwd.

void printDir(char *cwd)
{
  printf("file path after printdir call: %s\n",cwd);
  DIR *dirPtr = opendir(cwd);
  struct dirent *dirEnt;

  char *slash = "/";
  char *dot = ".";
  char *dotdot = "..";
  char *pathPrefix = malloc(sizeof(cwd) + sizeof(slash)+1);
  pathPrefix = strcat(cwd,slash);

  if (dirPtr != NULL)
  {
      while((dirEnt = readdir(dirPtr)) != NULL)
      {
          char *temp = dirEnt->d_name;
          if (strcmp(temp,dot) != 0 && strcmp(temp,dotdot) != 0)
          {

             char *tempFullPath = malloc(sizeof(pathPrefix) + sizeof(temp) + 1);
             tempFullPath = strcpy(tempFullPath, cwd);
             strcat(tempFullPath, temp);

             printf("file path before we try to openthedir: %s\n",tempFullPath);
             DIR *tempSubDirPtr = opendir(tempFullPath);

             printf("filePath after we try to open this shit: %s\n",tempFullPath2);
             if (tempSubDirPtr != NULL)
             {
                  printf("file path right before a recursive call: %s\n",tempFullPath);
                  closedir(tempSubDirPtr);
                  printDir(tempFullPath);
           }
           printf("%s\n",tempFullPath);
        }
    }
}
else
{

With the debugging printf()'s the console output is:

current working directory string after getcwd: /home/TTU/canorman/testUtil
file path after printdir call: /home/TTU/canorman/testUtil
file path before we try to openthedir: /home/TTU/canorman/testUtil/find.c
filePath after we try to open this shit: /home/TTU/canorman/testUtil/find.c
/home/TTU/canorman/testUtil/find.c
file path before we try to openthedir: /home/TTU/canorman/testUtil/find2.c
filePath after we try to open this shit: /home/TTU/canorman/testUtil/find2.c
/home/TTU/canorman/testUtil/find2.c
file path before we try to openthedir: /home/TTU/canorman/testUtil/find
filePath after we try to open this shit: /home/TTU/canorman/testUtil/find
/home/TTU/canorman/testUtil/find
file path before we try to openthedir: /home/TTU/canorman/testUtil/testDir
filePath after we try to open this shit: /home/TTU/canorman/testUA
file path right before a recursive call: /home/TTU/canorman/testUA
file path after printdir call: /home/TTU/canorman/testUA
Could not open specified working directory
/home/TTU/canorman/testUA/

So as you can see in the last few lines of the output that the file string goes from

/home/TTU/canorman/testUtil/testDir

to

/home/TTU/canorman/testUA

I can't find anything in the opendir(3) man pages about this happening. Any ideas as to why this going on.

Answer 1

There are some bugs related to string operations. Remember that str*() functions (strcat(), strcopy() etc.) put the result into buffer, which pointer is passed as the first argument. So:

1) About the code on beginning:

char *pathPrefix = malloc(sizeof(cwd) + sizeof(slash)+1);
pathPrefix = strcat(cwd, slash);

you should use mallocated buffer pointer as the first argument, of course you need to have the cwd copied here before strcat(). Below is proper code:

char *pathPrefix = malloc(sizeof(cwd) + sizeof(slash)+1);
strcpy(pathPrefix, cwd);
strcat(pathPrefix, slash);

2) Inside the first 'if' block in 'while' loop: you did it properly, but there is no need to assign 'tempFullPath' by value returned from 'strcpy()' - you already passed this pointer as the first argument, so the copying will be performed into this buffer. So, replace the:

char *tempFullPath = malloc(sizeof(pathPrefix) + sizeof(temp) + 1);
tempFullPath = strcpy(tempFullPath, cwd);
strcat(tempFullPath, temp);

with:

char *tempFullPath = malloc(sizeof(pathPrefix) + sizeof(temp) + 1);
strcpy(tempFullPath, cwd);
strcat(tempFullPath, temp);

3) Do NOT forget about freeing memory allocated by "malloc()' !!!! (every 'malloc()' must have corresponding 'free()').

Answer 2

This code is for sure wrong:

char *pathPrefix = malloc(sizeof(cwd) + sizeof(slash)+1);
pathPrefix = strcat(cwd,slash);

As noted by @Steve and @William, the sizeof are applying to the pointer instead of the character string, so it's allocating either 4 or 8 bytes each (plus one), and this is unlikely to be enough. strlen(s) counts the number of active characters in the string; you have to add +1 for the NUL byte yourself.

But a bigger problem is the use of strcat(), which appends the second string to the tail end of the first string, modifying that first string. This is NOT creating a new joined string for you, filling it into the newly-allocated (too small) memory.

This should behave better:

char *pathPrefix = malloc(strlen(cwd) + strlen(slash) + 1);
strcpy(pathPrefix, cwd);   // cwd goes to the start of the new buffer
strcat(pathPrefix, slash); // append the slash to the end of cwd

... then make similar changes in the rest of the code.

EDIT: When doing this kind of work, it's a good practice to const qualify any char * variables that you don't intend to modify (where the strings are readonly from your perspective). Doing this adds helpful documentation to your code, and the compiler will warn you of common mistakes.

Example:

void printDir(const char *cwd)
{
  ...
  const char *slash = "/";
  const char *dot   = ".";
  const char *dotdot = "..";

  ...

It's not legal to modify a string literal anyway, but had the cwd parameter been const-qualified, the compiler would have objected to strcat(cwd, slash) because it knows that the first parameter gets written to.

This doesn't really change the behavior of the code, but it's a good habit to get into. const is your friend.