1

I wrote the following code to reverse the string without using <algorithm> reverse . I wrote:

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


void reverse(char *str){
    int sizeStr=strlen(str);
    int firstChar,lastChar,temp;
    for(lastChar = (sizeStr - 1),firstChar = 0 ; lastChar>firstChar ; lastChar--, firstChar++){
        temp = str[firstChar];
        str[firstChar]=str[lastChar];
        str[lastChar] = temp;
    }
}
int main() {
    char *str;
    str = "myname";
    reverse(str);
    cout << str << endl; 
    return 0;
}

I am getting segfault at str[firstChar]=str[lastChar];. Please help me figure out my mistake.

NOTE: I declared char str[] = "myname" and worked perfectly. But as per your all explanations got to learn differences for *str and str[]. Thanks a lot. Goes a long way in helping a new programmer.

darthsidious
  • 2,851
  • 3
  • 19
  • 30
  • 3
    Just because you don't use `std::reverse` doesn't mean you shouldn't use `std::swap`. Anyway, same underlying problem as http://stackoverflow.com/questions/24874073/what-is-wrong-with-this-strcat-call-in-c – chris Jul 22 '14 at 03:29
  • 2
    By the way, `` is deprecated in C++. Pointing a `char *` to a string literal is also deprecated in C++ and should produce a compiler warning. In C++11, it is outright illegal. – chris Jul 22 '14 at 03:44

7 Answers7

5

You cannot modify a constant string literal.

For C++

Use std::string (thanks @chris for this). Use std::swap for swapping the characters. (See C++ string swap character places)

For C

malloc a buffer (with the length of the string), and strcpy the string to it, and then do the reverse for the newly allocated buffer.

Community
  • 1
  • 1
sampathsris
  • 21,564
  • 12
  • 71
  • 98
  • 2
    Since the tags changed to just C++, I'll have to suggest `std::string` over `malloc`. – chris Jul 22 '14 at 03:39
  • Thank you! This clears alot. Btw if I declare char str[] = "myname" the code works fine as it is the compliment of your explanation. But if I use *str = "myname" then I cant get the desired result as per your explanation. Than You @krumia !! – darthsidious Jul 22 '14 at 03:43
1

using std::string, you can use the method below to reverse a string

string reverse_string(string arg){
    int length = arg.length();
    char res[length];
    for(int i = length - 1; i >= 0; i--){
        res[length - (i+1)] = arg[i];
    }
    string result(res);
    return result;
}

Let me know if you have any further questions.

Bob Ezuba
  • 510
  • 1
  • 5
  • 22
0

The content of str is constant, you should use char pointer instead.

wshcdr
  • 935
  • 2
  • 12
  • 27
0

The reason is you have not allocated the memory to the character pointer str.

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


void reverse(char *str){
    int sizeStr=strlen(str);
    int firstChar,lastChar,temp;
    for(lastChar = (sizeStr - 1),firstChar = 0 ; lastChar>firstChar ; lastChar--, firstChar++){
        temp = str[firstChar];
        str[firstChar]=str[lastChar];
        str[lastChar] = temp;
    }
}
int main() {
    char *str;
    // Here use malloc to allocate memory to the character pointer str
    // Or instead of using a char pointer use a character array to initialize the string
    // char *str = malloc(strlen("myname") + 1)   or usr char str[10] = "myname"
    str = "myname";
    reverse(str);
    cout << str << endl; 
    return 0;
}
Santosh A
  • 5,173
  • 27
  • 37
0
#include <string.h>
#include <iostream>
using namespace std;


void reverse(char *str){
        int sizeStr=strlen(str);
        int firstChar,lastChar,temp;
        for(lastChar = (sizeStr - 1),firstChar = 0 ; lastChar>firstChar ; lastChar--, firstChar++){
        temp = str[firstChar];
        str[firstChar]=str[lastChar];
        str[lastChar] = temp;
   }
}

int _tmain(int argc, _TCHAR* argv[])
{
    char *str;
    str = "myname";
    char *str2 = new char [strlen(str)+1];
    strcpy(str2,str);
    reverse(str2);
    cout << str2 << endl; 
    delete [] str2;
    str2 = NULL;
    cout << str << endl; 
    return 0;
}
wshcdr
  • 935
  • 2
  • 12
  • 27
0

I think you might be going about this problem the hard way. If you use a std::string the problem becomes much easier. Below is a more generic C++1y solution which will be able to reverse many different types of containers. The container must override the [ ] operator. 

#include <iostream>

template <typename T>
auto rev(T &&a){
  for (auto i = 0; i < a.size()/2; ++i){
    std::swap(a[i], a[a.size()-1-i]);
  }
  return a;
}

int main(){
  std::string x = "hello";
  std::cout<< rev(x);
}
r-s
  • 1,035
  • 2
  • 11
  • 21
  • If you want to make this closer to `std::reverse`, that one takes a bidirectional iterator rather than a random access iterator (of course yours doesn't take an iterator, but it's still random access). Taking two bidirectional iterators makes it easy to increment one and decrement the other while calling `std::iter_swap` along the way. If you want to wrap that with something taking a container, it's also trivial to take the container and use `std::begin` and `std::end` on it to pass to the iterator version. – chris Jul 22 '14 at 03:50
  • Your are right chris. The question is a bit funny since the best solution is just to use std::reverse, and if its not availible for some reason, we likely are best off re-writing it. – r-s Jul 22 '14 at 03:52
  • Yeah, using the one that's already there is great, or some range library that takes a single range object instead of two iterators. Or even something like Boost's adaptors, where I believe `std::cout << (x | reversed);` would work. – chris Jul 22 '14 at 03:53
0
#include <string.h>
#include <iostream>

void myreverse(std::string &s) {
    size_t begin = 0, end = s.length();

    while ((begin != end) && (begin != --end))
        std::swap(s[begin++], s[end]);
}

int main() {
    const char *str = "myname";
    std::cout << str << std::endl; 

    std::string newstr(str);
    myreverse(newstr);
    std::cout << newstr << std::endl; 
    return 0;
}
denisvm
  • 720
  • 3
  • 11