views:

504

answers:

4

Using C, I'm trying to concatenate the filenames in a directory with their paths so that I can call stat() for each, but when I try to do using strcat inside the loop it concatenates the previous filename with the next. It's modifying argv[1] during the loop, but I haven't worked with C in a long time, so I'm very confused...

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <dirent.h>
#include <string.h>

int main(int argc, char *argv[]) {
 struct stat buff;

 int status;

 if (argc > 1) {
  status = stat(argv[1], &buff);
  if (status != -1) {
   if (S_ISDIR(buff.st_mode)) { 
     DIR *dp = opendir(argv[1]);
     struct dirent *ep;
     char* path = argv[1];
     printf("Path = %s\n", path);

     if (dp != NULL) {
       while (ep = readdir(dp)) {
       char* fullpath = strcat(path, ep->d_name);
       printf("Full Path = %s\n", fullpath);
     }
     (void) closedir(dp);
   } else {
      perror("Couldn't open the directory");
   }
 }

  } else {
   perror(argv[1]);
   exit(1);
  }
 } else {
   perror(argv[0]]);
                exit(1);
 }

 return 0;
}
A: 

You must not try to increase the size of argv[1] by strcat'ing to it (and changing it at all is a bad idea) - that will cause undefined behaviour. Instead, copy argv[1] into a suitable sized buffer and work with that:

char path[1000];   // or some suitable size;
strcpy( path, argv[1] );
anon
Don't you mean strcpy? path might not be zero filled
Mark
@Mark Correct - silly typo.
anon
A: 

Where is the memory that you are copying to? path is allocated on the stack to contain the arguments you need yo allocate the memory yourself e.g.

char path[1024] ;   // or some other number
strcpy( path, argv[1] );
// then separator
strcat( path, "/" ) ; // or "\\" in Windows
strcat( path, ep->d_name);

In production code se strncat etc to stop overflows

Mark
A: 

The problem is that the line

char* fullpath = strcat(path, ep->d_name);

keeps appending the name of the current file to path. Try creating a new string for the full path each time, like

char* fullpath = calloc(strlen(path) + strlen(ep->d_name) + 1);
strcat(fullpath, path);
strcat(fullpath, ep->d_name);
/* .. */
free(fullpath);
Frerich Raabe
+2  A: 

You shouldn't modify argv[i]. Even if you do, you only have one argv[1], so doing strcat() on it is going to keep appending to whatever you had in it earlier.

You have another subtle bug. A directory name and file names in it should be separated by the path separator, / on most systems. You don't add that in your code.

To fix this, outside of your while loop:

size_t arglen = strlen(argv[1]);

You should do this in your while loop:

/* + 2 because of the '/' and the terminating 0 */
char *fullpath = malloc(arglen + strlen(ep->d_name) + 2);
if (fullpath == NULL) { /* deal with error and exit */ }
sprintf(fullpath("%s/%s", path, ep->d_name);
/* use fullpath */
free(fullpath);
Alok