Charles Geter
Charles Geter

Reputation: 11

Need help figuring out why my Reverse String c++ program is breaking

I'm trying to reverse a string just as a simple exercise to prepare myself for interviews. I wanted to write a program that takes keyboard input and reverses it, in c++. For some reason I am getting an EXC_BAD_ACCESS error (in Xcode) at the line "cin >> myString;" I suspect that I am doing something pretty stupid, but I could really use some help in finding out what's wrong, and how I should fix it. Thanks!

#include <iostream>
#include <cstring>
#include "stdlib.h"

using namespace std;

int main(int argc, const char * argv[])
{
    char *myString = nullptr;
    char *revString = nullptr;
    cout << "Please enter a string: ";
    cin >> myString;

    cout << endl;

    size_t myLength = strlen(myString);

    for (int i = 0; i < myLength; i++)
    {
        revString[i] = myString[myLength -1 - i];
    }

    cout << "Your string, reversed, has become: ";
    printf(revString);
    cout << endl;
}

Upvotes: 1

Views: 752

Answers (4)

hmjd
hmjd

Reputation: 122001

No memory has been allocated for myString or revString. You need to allocate memory using new[] or use std::string. If you use new[], or use a stack allocated array of char, then you will need to limit the number of characters read to avoid buffer overrun. Using std::string removes this responsibility from you as std::string will dynamically grow to the required size.

Unsure if this is a learning exercise or STL algorithms are not permitted but if they were you could use std::reverse() to reverse a std::string (see example here http://ideone.com/7LZHs).

Upvotes: 5

JoeFish
JoeFish

Reputation: 3100

The problem is you're trying to cram data into a pointer that doesn't point to memory you can use.

This line declares a pointer to a character and sets it to nullptr:

char *myString = nullptr;

But that's it - it doesn't point at any writable memory, so there's nowhere valid for the input to go. cin doesn't do any sort of memory allocation for you.

One way to fix it is to statically allocate your string:

char myString[SOME_SIZE] = "";

Where SOME_SIZE is a constant value big enough to hold your input string.

Or you can declare it as a std::string instead, as was suggested by another poster. In C++ this is the safer/better way to do it to avoid any possible overflows.

std::string myString;

Upvotes: 0

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726839

There are at least two problems with your code: you are writing to a character buffer that you did not allocate, and you are constructing a palindrome from the second half of the string, rather than reversing it.

Since this is C++, you shouldn't use C strings, and prefer C++ strings instead (std::string is the type that you need). You should also swap the characters at the two ends of the string, rather than simply assigning the character from the ending part to a character from the initial part of the string. The most fitting function for swapping characters is std::swap.

string str = "quick brown fox jumps over the lazy dog";
for (int i = 0, j = str.size()-1 ; i < j ; swap(str[i++], str[j--]))
    ;
cout << str;

Upvotes: 2

ThirdOne
ThirdOne

Reputation: 1248

You need to allocate memory for myString. Use a static array or a new statement.

char myString[512];
char * myString = new char[512];

Upvotes: 0

Related Questions