1

I've been trying to debug this for a while, still can't figure out why this causes a stack smashing error (I think the error code is 6, or abort. Essentially this function takes a directory, opens a file and then puts that file into a function so it can use the file, and then outputs the number of times it goes through the function.

int map(char* dir, void* results, size_t size, int (*act)(FILE* f, void* res, char* fn))
 {
      printf("%s\n", dir);
      char copyDirectory[strlen(dir)+1];
      //adds the slash
      strcpy(copyDirectory, dir);
      strcat(copyDirectory, "/");
      //before the psuedocode, get all the files in the directory
      int numFiles = nfiles(copyDirectory);
      DIR* directory = opendir(copyDirectory);
      //if there aren't any files, then we exit
      if(numFiles == 0)
      {
           closedir(directory);
           return -1;
      }
      //reads the file from the directory
      struct dirent* readFile = readdir(directory);
      int output = 0;
      while(readFile!=NULL)
      {
           if(readFile->d_type==DT_REG)
           {
               //step 2: obtain filepath
               char* fileName = readFile->d_name;
               int filePathLength = strlen(dir) + strlen(fileName) + 1;//add one for the slash
               char filePath[filePathLength];
               memset(filePath, 0, filePathLength); //allocat ememory for file path
               strcpy(filePath, strcat(dir, fileName));
               //step 3: open file
               FILE* file = fopen(filePath, "r");
               //if the file is unreachable, exit
               if(file==NULL)
               {
                    closedir(directory);
                    return -1;
               }
               //step 4: perform some action and store result
               strcpy(dir, copyDirectory);
               act(file, results, fileName);
               //step 5: close file
               fclose(file);
               //to go through loop: increment the readFile
                    ++output;
                }
                readFile = readdir(directory);
      }
      closedir(directory);
      return output;
 }

Map function with the example.

int map(char* dir, void* results, size_t size, int (*act)(FILE* f, void* res, char* fn))
{
     char* copyDirectory = strdup(dir);
     DIR* directory = opendir(dir);
     int output = 0;
     struct dirent* readFile = readdir(directory);
     while(readFile!=NULL)
     {
         if(readFile->d_type==DT_REG)
         {
             //step 2: obtain filepath
             char* fileName = readFile->d_name;
             int filePathLength = strlen(dir) + strlen(fileName) +2;//add one for the slash
             char filePath[filePathLength+1];
             memset(filePath, 0, filePathLength); //allocat ememory for file path
             strcpy(filePath, strcat(dir, fileName));
             //step 3: open file
             FILE* file = fopen(filePath, "r");
             //if the file is unreachable, exit
             if(file==NULL)
             {
                 closedir(directory);
                 return -1;
             }
             //step 4: perform some action and store result
             strcpy(dir, copyDirectory);
             act(file, results, fileName);
             //step 5: close file
             fclose(file);
             //to go through loop: increment the readFile
             ++output;
         }
         readFile = readdir(directory);
     }  
     closedir(directory);
     return output;
}
//Sample Map function action: Print file contents to stdout and returns the number bytes in the file.
int cat(FILE* f, void* res, char* filename) {
    char c;
    int n = 0;
    printf("%s\n", filename);
    while((c = fgetc(f)) != EOF) {
        printf("%c", c);
        n++;
    }
    printf("\n");
    return n;
}
int main(int argc, char const *argv[])
{
    char directory[]= "../rsrc/ana_light/";
    size_t size = 100;
    void* results[500]; 
    int mapCat = map(directory, results, size, cat);
    printf("The value of map is %d.\n", mapCat);
    return EXIT_SUCCESS;
}

Where this fails is after it has executed and it prints to the output. The function should print out the contents of the files you have. The directory list needs to have a "/" at the end. Currently it prints the file contents and exits with the value of how many files it read, but it drops off after it exits with the stack smashing error.

EDIT1: Edited the code to reflect the changes I've made.

EDIT2: Done according to the MCVE standards, I think? Should run if I'm not mistaken.

  • At `char copyDirectory[strlen(dir)+1]; strcpy(copyDirectory, dir);` the `strcpy()` uses every allocated byte, but then you do `strcat(copyDirectory, "/");` writing beyond the end of the array. That's smashing the stack. – Jonathan Leffler Sep 11 '16 at 01:47
  • Hm, even before this happened, it was smashing the stack. Even if I hard code the "/" into the main method, it produces an error. – Razgrizaces Sep 11 '16 at 01:58
  • 1
    Please review how to create an MCVE ([MCVE]), and then add the (minimum) necessary code to make it possible for us to exercise what you show. You should add this as a new version of your code — leave the original visible since you have an answer which addresses real issues in your original code. Which compilation options are you using with which compiler on which platform? Are you using [`valgrind`](http://valgrind.org/) to help diagnose problems? Could you? (It isn't available on all platforms.) – Jonathan Leffler Sep 11 '16 at 02:06
  • One serious question for you: if you omit the `act` argument and the call to the function, do you still get the crash? If so, omit that; it will simplify your MCVE considerably. – Jonathan Leffler Sep 11 '16 at 02:07
  • Sorry, had not heard of the MCVE standards. Fixed the code to account for that. And it needs to work with the act portion. It does work with it, but at the end it terminates with the stack smashing detected error. – Razgrizaces Sep 11 '16 at 02:25
  • You seem to have a solution; that's good. Note that when you're debugging a problem like this, it is no longer automatically necessary to use `act` to see the bug. You're right; you'd not be able to submit the code without it, but the MCVE isn't the code you're going to submit. It's a toy program that shows the problem without all the encumbrances of the full program. It's a distillation of the problem your facing in debugging — not the homework problem you're trying to complete. It's no longer an issue this time. (Not having come across MCVE before isn't a problem; the URL is to educate). – Jonathan Leffler Sep 11 '16 at 03:33

1 Answers1

2

First problem: change

    char copyDirectory[strlen(dir)+1];

to

    char copyDirectory[strlen(dir)+2];

Second problem: change

       char filePath[filePathLength];

to

       char filePath[filePathLength+1];

Third problem (change appears to have not been there on first read):

     //strcpy(copyDirectory, dir);
     strcat(copyDirectory, dir);

The commented out code was correct:

    strcpy(copyDirectory, dir);

You've been forgetting spaces for trailing null characters.

Fourth problem: You forgot to handle opendir failing.

Fifth problem: this code is wrong:

        memset(filePath, 0, filePathLength); //allocat ememory for file path
        strcpy(filePath, strcat(dir, fileName));

change to:

         strcpy(filePath, copyDirectory);
         strcat(filePath, fileName);

Don't write to your input variables here. This is a very bad idea.

After undoing the (leaky) strdup for copyDirectory and putting your very innovative local buffer back I was able to get the code to run through to completion.

Joshua
  • 40,822
  • 8
  • 72
  • 132
  • I think I understand, I need to add another space for the null terminator, but it doesn't seem to help, the error still persists. – Razgrizaces Sep 11 '16 at 01:55
  • 1
    Attempt an MVCE then. I need something to run to get farther. – Joshua Sep 11 '16 at 02:04
  • Well, if you put it through the main it still goes through, it just terminates after main. Whoops, not done. I'll edit the code in a sec to show where the error seems to be happening and with an example. – Razgrizaces Sep 11 '16 at 02:12
  • Edited the code. You can replace the directory with any directory, just end it with "/". It prints out fine, just after the end of the main it terminates with the error code. – Razgrizaces Sep 11 '16 at 02:23
  • Thank you very much! I'll fix the other things you mentioned, but yes, this does eliminate the error! I appreciate it a lot, this took probably ~10+ hours of me banging my head against a wall trying to figure it out. As for the local buffer, I'm not using a local buffer for the actual project, I just did it for simplicity. – Razgrizaces Sep 11 '16 at 02:39