1

I am currently writing a command line "parser" so to speak, and so far it has been working until I tried a few ways to add options/parameters. 

void parser::nextCom() { cout << parser::prompt; // parser::prompt = "> " string com; getline(cin, com); char the_command[5]; // i want this to store the command e.g. "go" given that go is a command for (int i = 0; i < com.size(); i++) { if (com[i] == ' ') break; else the_command[i] = com[i]; } cout << the_command << endl; }

The command is copied but some very unwanted characters show up when in print the_command to the console.

This is what I get if I pass "go north" as a command:

goÌÌÌÌÌÌÌÌÌÌÌÌÌÌØNi

I am not too sure about char arrays in C++, but I don't know how I am getting this output. Any help at all will be appreciated. Any questions about the code or if you need more of my code, just comment, thanks in advance

Improve this question

5 Answers 5

Reset to default
4 
cout << the_command << endl;

When you print a char array like this, characters continue to be inserted until the null character \0 is found in the string.

Before you start copying characters from com to the_command, the array is completely uninitialized. I'll represent these unknown characters with question marks (but of course, they're probably not actually question marks):

? ? ? ? ?

This means you have no idea what the values of the chars in the array will be. You then copy only the characters g and o from the_command into com, so your array now contains:

g o ? ? ?

So when you attempt to output this array, the output stream doesn't know when to stop. You need to make sure you insert an \0 after the o. One way to do that would be:

for (int i = 0; i < com.size(); i++) { if (com[i] == ' ') { the_command[i] = '\0'; break; } else the_command[i] = com[i]; }

This will leave the array like so:

g o \0 ? ?

However, you'd be much better off just sticking to std::string. I don't want to think about the trouble you'll have with this array that could just be avoided. Here's how I would write your function:

void parser::nextCom() { std::cout << parser::prompt; std::string command_line, command; std::getline(cin, command_line); std::stringstream command_line_stream(command_line); command_line_stream >> command; if (command == "go") { std::string direction; command_line_stream >> direction; go(direction); } }
Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

i wanted to make get the command and use if statements or switch statement to determine which function to run
well you have a point that i'd have trouble with the char array, just wasn't sure how to seperate the command using a string, does string stream do that for me?
@PurityLake std::stringstream makes it easy. You could do it with a std::string too.
Why is is std::stringstream being used in place of std::istringstream?
@David Either will do. You're probably right that std::istringstream is more appropriate here.
3

You're not null-terminating the_command after the last character is read. Or doing any bounds checking.

Please, use std::string instead.

Improve this answer

Comments

3

Change the code to:

if (com[i] == ' ') { com[i] = '\0'; break; }

This will ensure there is a null terminator at the end of your char array. The reason you are seeing garbage is because std::cout will happily print characters until it sees a null terminator.

Improve this answer

Comments

1

this is because you have a buffer overflow in your code. you copied an indeterminate length string into a char[5] buffer... basically, your loop is copying as many bytes as determined by the input string, into past the end of the char[5] array, which is no longer null terminated, so "cout" is just reading until it finds null bytes.

Improve this answer

3 Comments

the way I was looking at it was that the first command will be no bigger than 5 characters
Well, theoretically, if (com[i] == ' ') break; should prevent the buf overflow for the input "go north". (The code itself is still unsafe of course as you can't guarantee that condition for all input you might receive.)
Nice catch, Either way though, the buffer isn't null terminated, causing cout to read too far.
1

Basically the_command[5] contains garbage since is not initialized and doesn't contains the character terminator. You can clear it first, and you'll be fine

for (i = 0; i < 5; i++) { the_command[i] = 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.