1

So, I have a function that reads strings from a file and pushes them into a vector of strings called words. I have a Quicksort function which essentially would sort the whole vector, but when I call the Quicksort function, it gives me this error "Thread 1: EXC_BAD_ACCESS (code=2, address=0x7ffeef3ffff8)" and in the Virtual Box, I am using Debian 9, it gives me a Segmentation Error.

  int main()
  {
    int time[5];
    vector<string> words;
    words=ReadFromFile();
    auto start = std::chrono::system_clock::now();
         quicksort(words, 0, words.size()-1);
         auto end = std::chrono::system_clock::now();
         std::chrono::duration<double> elapsed_seconds = end-start;
         cout<<endl;
         time[2]=elapsed_seconds.count();
         cout<<" Elapsed time: " << elapsed_seconds.count() <<"\n ";
         cout<<endl;
   }

I want to get the time that it takes the function to sort all the words.

 void quicksort(vector<string> words, long int from, long int to) //goes into infinite loop
   {
     if (from > to)
       {
         return;
       } 
     long int p = partition(words, from, to);
     quicksort(words, from, p);
     quicksort(words, p + 1, to);

      }

   long int partition(vector<string> words, long int from, long int to)
   {
      string pivot = words[from];
      long int i = from - 1;
      long int j = to + 1;
      while (i < j)
      {
    i++;while (words[i] > pivot){i++;}
    j--;while (words[j] < pivot){j--;}
    if (i < j){std::swap(words[i],words[j]);}
       }
     return j;
    }

My read from file function is this-

vector<string> ReadFromFile(){
 vector<string> words;
 ifstream read;
 read.open("macbeth.txt");
 if(!read.is_open())
 {
     cout<<"Error Opening file"<<endl;
 }
 else
 {
     while(!read.eof())
     {
         string line;
         getline(read,line);
         stringstream Curstr(line);
         while(Curstr>>line)
         {
             words.push_back(line);
         }
     }
 read.close();
 }
 return words; 
}

So I get a EXE_bad_access, and the recursion runs for about 50,000 times.

@EDIT Using the debugger, I found out that pivot is not getting the value from words[from]. It shows that string pivot = "" instead it should have the value like string pivot = words[from] //MACBETH which is the value of words[from] where from = 0 when the function is called the first time. Now, how do I get the value into pivot so it can divide the vector and perform the sort.

  • 1
    And when you used your debugger to run your program, what did you see? This is what a debugger is for. If you don't know how to use a debugger this is a good opportunity to learn how to use it to run your program one line at a time, monitor all variables and their values as they change, and analyse your program's logical execution flow. Knowing how to use a debugger is a required skill for every C++ developer, no exceptions. With your debugger's help you should be able to quickly find all bugs in this and all future programs you write, without having to ask anyone for help. – Sam Varshavchik Mar 07 '20 at 19:16
  • This is an excellent candidate for debugging. – JohnFilleau Mar 07 '20 at 19:16
  • You shouldn't be passing the vector by value (which makes a copy). You should pass by reference (`vector &words`) so that the changed vector is retained by the caller. – 1201ProgramAlarm Mar 07 '20 at 19:26
  • The shown code does not compile, because `time` is undeclared or should fail because `time[2]` is trying to index a function (although gcc only warns for that by default) and I have no idea what `time[2]=elapsed_seconds.count();` is supposed to do. Please provide a *complete* [repro], look at and fix all warnings given to you by the compiler, and use a debugger to step through your program and figure out the logic error. – walnut Mar 07 '20 at 19:30
  • Not only is this a good candidate for a debugger, but also you already know what to look for: infinite recursion. Just keep an eye on the parameters being passed to `quicksort` and start your line-by-line debugging when the parameters start repeating. – JaMiT Mar 07 '20 at 19:56
  • I'll even give you a simple case to test: `vector words{1, "Word"};` and skip the read from a file. – JaMiT Mar 07 '20 at 20:01

1 Answers1

0

God... Don't do that. You are passing vector by copy while invoking those functions. Try to profile it and check if you don't run out of memory... (you probably do...)

void f(vector<string> words){
  // here only local words are modified, the vector that you passed from caller function is NOT affected
} 

^ the COPY of vector is passed. What you want to do is pass it by reference.

void f(vector<string> &words) {
// here you operate on original words vector
}

Try to fix that first and see if that helps (it should, mac can throw this error on memory limit as I recall properly)

here is an explanation how it works

@EDIT Try to run it with default sort() from stl. Also check if you don't overrun bounds of vector when calculating i/j - add some asserts to i and j : assert(0 <= i && i < words.size()) to check for correctness.

@EDIT2 It's not like stack is a place to debug your code for you... But I did a quick little investigation on some edge cases so listen:

  1. use vector<string>& words as function arguments
  2. reading files isn't as easy as it seems. Even though yours SOMEHOW works, don't use eof(). Here: .eof() loop not working
  3. i++;while (words[i] > pivot){i++;} is essentialy do{i++;}while(words[i] > pivot)
  4. ^ and it doesn't work because the condition is never met (first words[i] IS pivot) so i always stays the same
  5. it loops infinitely for array of size 1 for example ["a"] (returned pivot is always 0)
  6. you probably know the concept of quicksort but your implementation is wrong. Go check https://www.geeksforgeeks.org/quick-sort/ to learn how it should be done.
  7. Don't use long unless you know what you are doing. Either you want int or long long.
  8. This is... basic debugging / analysing code... Seems like you didn't put ANY effort to analyze the code. Stack is not a place for others to solve your problems because you are too lazy.
  9. Go and read some tutorials about C++, because you don't have strong fundamentals yet.
Tooster
  • 322
  • 3
  • 14
  • Passing the vector by reference instead of by value will at best increase the number of function calls before the infinite recursion crashes the program. – JaMiT Mar 07 '20 at 19:58
  • @JaMiT I tried your test case, it still gave the error. I think it has something to do with my algorithm but I can't seem to understand what I am doing wrong. Is it because I am treating the vector as an array? – Vibhor Sagar Mar 08 '20 at 19:02
  • Vectors are meant to be used as arrays. Just on steroids with dynamic size, push/pop mechanics etc etc. @VibhorSagar did you check if you don't access vector outside of bounds ? The best way to check when your program crashes would be to use a debugger and check which line produces the error. It's the fastest method to find potential problems... – Tooster Mar 08 '20 at 19:08
  • I implemented the std::sort() from STL, it works perfectly. I know std::sort() is a type of quicksort. But then, how do I make my quicksort function work with my code? – Vibhor Sagar Mar 08 '20 at 19:08
  • @Tooster I honestly don't know how to use the Xcode debugger. When I go to the debugger it highlights the line where the recursion is happening until the memory caps. – Vibhor Sagar Mar 08 '20 at 19:11
  • @VibhorSagar The test case was *intended* to still give you the error. That's why I gave it to you. It's a simplified case to make it easier for you to learn how to use your debugger. (Set a breakpoint in `quicksort` and watch its parameters.) – JaMiT Mar 08 '20 at 21:14
  • @Tooster regarding your comment "profile it and check if you don't run out of memory." which flag of Valgrind could show running out of memory? – sepideha Nov 14 '20 at 02:43
  • 1
    @sepideha I'm not a Valgrind expert, so I will redirect you to [that exhaustive answer](https://stackoverflow.com/questions/5134891/how-do-i-use-valgrind-to-find-memory-leaks). Aside from Valgrind, you could use AddressSanitizer and LeakSanitizer from GCC with some kind of `-fsanitize=leak` flag(if you compile under gcc or clang). – Tooster Nov 15 '20 at 00:04