CODEWITHSUNDEEP
c++pointers
gnuShark
gnuShark

Reputation: 13

C++ pointer issues

i am trying to create a simple (modularized) c++ program that reads the users input and spits it back out.

#include "stdafx.h"
#include <iostream>
#include <fstream>
#include <string>

using namespace std;

void _printOut(char * output)
{
    cout << output << endl;
}
char * readUserInput()
{
    char userInput[256];
    cin >> userInput;
    return userInput;
}

int _tmain(int argc, _TCHAR* argv[])
{
    _printOut("Enter your name: ");
    char * userName = readUserInput();
    _printOut("Hello");
    _printOut(userName);
    system("pause");
    return 0;
}

Enter your name: aaaa Hello ╠╠╠╠╠╠╠╠ Press any key to continue . . .

if i print out the userInput variable in the readUserInput function it prints out what is inputted. However trying to print out the userInput variable store as userName in the _tmain function results in a incomprehensible sequence of chars being printed out. ie. ╠╠╠╠╠╠╠╠. By my best guess, this could be caused by pointer issues, but as far as I can tell I am referencing everything correctly.

Debugging this code: at this line: _printOut("Hello"); in method: _tmain [userName = "abcdefg"] at this line: _printOut(userName); in method _tmain [userName = "†UX"]

so I am wondering how the value of username is changing when it isn't assigned or manipulated between the two lines.

Upvotes: 1

Views: 209

Answers (5)

BoBTFish
BoBTFish

Reputation: 19757

char * readUserInput()
{
    char userInput[256];
    cin >> userInput;
    return userInput;
}

The char userInput[256]; array only exists during the function call. Once you hit the bottom of the function it ceases to exist, and you return a pointer to some garbage memory. This is called local scope.

And anyway, what if someone has a reaally long name (longer than 255 characters).

Consider using std::string, which will solve both problems.

std::string readUserInput()
{
    std::string inp;
    std::cin >> inp;
    return inp;
}

and

void printOut (const std::string& toPrint)
{
    std::cout << toPrint << '\n';
}

(Also, and this is less important, the name _printOut is not allowed in this context, because of the leading _. See here, although it might go over your head if you are a beginner.)

Edit An even better way to go is to use std::getline to read a whole line at a time into a std::string. However, because of the way they treat spaces, and in particular '\n' newline characters, getline(...) and cin>>... don't play nicely together. It's usually just best to pick one and stick with it throughout your program. Here's how readUserInput() would look:

std::string readUserInput()
{
    std::string line;
    std::getline(std::cin, line);

    return line;
}

This way if a user enters a name containing whitespace (e.g. "BoB T. Fish") you will read the whole name, rather than just "BoB" (and then leaving the rest to confuse you the next time you read).

The reason this can be iffy to mix cin>>... with getline is that cin>>... will read as much as it can up to whitespace, then leave the rest behind. So apart from mayb emissing someone's surname, if they enter a name without spaces, it will just leave the last newline character on the input stream. Then when you come along and do getline, you don't get the next line of user input. You get the empty line that has been left behind. If you had instead used cin>> again, the newline would have been ignored. e.g. Consider this user input:

Hello\n
World\n

If you do the first read with cin>>, you get "Hello" in your program, and left with

\n
World\n

If you then do the second read with getline, you get "" in your program, and left with

World\n

Upvotes: 4

SysAdmin
SysAdmin

Reputation: 5575

you are using a pointer to a buffer (In stack) which is already out of scope.

Try this:

#include <iostream>
#include <fstream>
#include <string>

using namespace std;

void _printOut(char * output)
{
    cout << output << endl;
}
char * readUserInput()
{
    //Allocate in Heap, instead of stack
    char* userInput = new char[256];
    cin >> userInput;
    return userInput;
}

int main(int argc, char* argv[])
{
    _printOut("Enter your name: ");
    char * userName = readUserInput();
    _printOut("Hello");
    _printOut(userName);
    system("pause");
    return 0;
}

A better way would be to use std::string

Upvotes: 0

Mario
Mario

Reputation: 36487

Your problem here is a scoping issue:

char userInput[256]; defines a local variable that is only valid within its own scope (between ist set of {} brackets).

You essentially return a valid pointer that becomes invalid once you leaves the function, because it's freed.

You're violating a basic rule: never return pointers to local (non-static) variables.

To fix this, make your userInput static or return a new string (create a new string every call using new) or use copyable objects rather than pointers (like std::string).

Upvotes: 0

Luchian Grigore
Luchian Grigore

Reputation: 258548

char userInput[256]; will be destroyed when readUserInput exits, so the pointer you return is invalid.

Use std::string instead.

Or allocate the variable dynamically, or pass an automatic variable as parameter.

Upvotes: 2

bjskishore123
bjskishore123

Reputation: 6342

userInput variable gets destroyed once it goes out of scope.

Local variables get stored on stack. Once the function execution completes, the variables get destroyed.

Either you need to use dynamically allocated char pointer(Stores on Heap) or std::string

Upvotes: 1

Related Questions