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.

Improve this question
2
  • 3
    Just because you don't use std::reverse doesn't mean you shouldn't use std::swap. Anyway, same underlying problem as stackoverflow.com/questions/24874073/… Commented Jul 22, 2014 at 3:29
  • 2
    By the way, <string.h> 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. Commented Jul 22, 2014 at 3:44

7 Answers 7

Reset to default
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.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Since the tags changed to just C++, I'll have to suggest std::string over malloc.
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 !!
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.

Improve this answer

1 Comment

Variable length arrays are not standard C++.
0

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

Improve this answer

1 Comment

You'll have to elaborate. The OP is currently using a char pointer.
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; }
Improve this answer

Comments

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); }
Improve this answer

3 Comments

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.
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.
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.
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; }
Improve this answer

1 Comment

delete str2; is undefined behaviour. Variable length arrays are not standard C++.
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; }
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.