CODEWITHSUNDEEP
c++segmentation-fault
blaedj
blaedj

Reputation: 342

Segmentation fault error

`I am trying to write a program that reverses two strings, I though I had it done pretty well but when I run it, the program runs till line 26, then I get a segmentation fault error. The program compiles fine. I am wondering if there is a simple or obvious problem in my functions that I am not seeing, Any help would be appreciated!!

Thanks in advance

#include <iostream>
#include <string>
using namespace std;

// Reversing the characters in strings.

void reverse(string str);
void swap(char * first, char *last);

int main() {
    // declarations and initialization
    string str1;
    string str2;

    cout << "Please enter the first string of characters:\n";
    cin >> str1;

    cout << "Please enter the second string of characters:\n";
    cin >> str2;

    cout << "The strings before reversing are:" << endl;
    cout << str1 << " " << str2 << endl;

    // reverse str1
    reverse(str1);
    // reverse str2
    reverse(str2);

    // output
    cout << "The strings after reversing: " << endl;
    cout << str1 << " " << str2 << endl;

    return 0;
}

void reverse(string str) {
    int length = str.size();

    char *first = NULL;
    char *last = NULL;
    first = &str[0];
    last = &str[length - 1];
    for (int i = 0; first < last; i++) {
        swap(first, last);
        first++;
        last--;
    }
}

void swap(char *first, char *last) {
    char * temp;

    *temp = *first;
    *first = *last;
    *last = *temp;
}

Upvotes: 4

Views: 721

Answers (5)

badmaash
badmaash

Reputation: 4845

Again, others have pointed out the what the problem is and would like to show you this:

void reverse(std::string & str) {     
 for (int i = 0, last = str.size() - 1, lim = str.size() / 2 ; i < lim;) {
  std::swap(str[i++], str[last--]);
 } 
}

I have not tested it thoroughly though.

Upvotes: 0

Seth Carnegie
Seth Carnegie

Reputation: 75150

You're passing the strings by value, which means only a local copy of the string will be reversed in the reverse function. You should pass them by reference.

Also, don't alter the string's memory directly. Use operator[] like this:

for (size_t beg = 0, size_t end = str.size() - 1; beg < end; ++beg, --end)
    str[beg] = str[end];

So all together:

void reverse(string& str); // prototype

....

void reverse(string& str) { // note the & which is pass by reference
    int length = str.size();

    for (size_t beg = 0, size_t end = str.size() - 1; beg < end; ++beg, --end)
        str[beg] = str[end];
}

And as stated by Mooing Duck, the place you're probably crashing from is dereferencing a pointer which has a garbage value here:

char * temp;
*temp = ...

You're trying to assign some random memory a value, which is probably segfaulting you.

Upvotes: 2

Marlon
Marlon

Reputation: 20312

In your swap function, you are assigning a value to *temp when temp is not pointing to anything (it's uninitialized). Thus, your segmentation fault.

You want this:

void swap(char* first, char* last)
{
    char temp = *first;
    *first = *last;
    *last = temp;
}

Upvotes: 3

netcoder
netcoder

Reputation: 67745

The other answers are valid in regards to the segfault cause.

I just think you may be interested in knowing that you can reverse a string easily using std::string's reverse_iterator:

std::string reverse(std::string str) {
    std::string out;
    for (std::string::reverse_iterator it = str.rbegin(); it != str.rend(); it++) {
        out += *it;
    }
    return out;
}

So, calling:

reverse("foo");

...will return oof.

Upvotes: 2

Mooing Duck
Mooing Duck

Reputation: 66961

I don't know where line 26 is, but 

char * temp;
*temp = ...

is not valid. temp should be pointed at a char, or (better yet) rewrite the function to where temp is a char.

Seth Carnegie observes that you'll have to pass the strings by reference if you want to modify the originals.

void reverse(string& str) { //pass by reference, so origional is modified

Upvotes: 6

Related Questions