Endless do while loop in C++ code?

Question

char input[256];
do{
    cout<<"..."; //prompt
    scanf("%s",&input5); //user inputs string
    if(strcmp(input5,"block")==0)
    {}                                 //if the user types block, then it finishes the loop and goes to the cout at the bottom
    else if(strcmp(input5,"attack")==0)
    {
        cout<<"..."<<endl;
    }
    else if(strcmp(input5,"look")==0)
    {
        cout<<"..."
    }
    else
    {
        cout<<"..."<<endl;
    }
}while(strcmp(input5,"block")!=0); //loop ends when block is typed
cout<<"...";

I am having issues with my do while loop. I am doing a project for school that involves a text adventure kind of game. The user is prompting how to respond to an attack. The desired command is "block", which will move the user on to the next sequence. When "block" is typed into the scanf, it endlessly loops and prints what is in the "else" condition. I don't see what the problem is and all feedback is appreciated.

What is `input5`? I think you should have `scanf("%s",input5)` -- no '&'. — G.M., Apr 14 '17 at 13:22
If you'd learn to use a debugger, you could have figured this out yourself in less time than it took to start your browser, navigate here, and find the *Ask Question* button at the top of the page. Learning to use a debugger to trace through the logic of your code and evaluate variable content is one of the most important skills you'll ever learn as a programmer. It's never too soon to start. — Ken White, Apr 14 '17 at 13:25
This is C++. Use `std::cin >>` instead of `scanf`, and use `std::string` instead of `strcmp` and friends. — Angew is no longer proud of SO, Apr 14 '17 at 13:25
No, `input5` cannot be a char. The function `strcmp` expects a pointer to char and the compiler would raise a compilation error otherwise. — , Apr 14 '17 at 13:40
`scanf("%s", input5)`, remove the `&`. See the dup I linked. — AndyG, Apr 14 '17 at 13:48

Cl9 · Answer 1 · 2017-04-14T13:46:46.180

2

I just tried your code and it works fine (though I removed the & in the scanf), and created 'input5' as a char array.

Though that aside, there's a few things that you might want to change. I'd stick to using 'cin' instead of scanf, as you're mixing C and C++. That would allow you to use a 'string' for 'input5', and compare them using the '==' operator, which is quite a bit cleaner. Maybe think of a more descriptive name than 'input5' too, as if you've got lots of 'inputX' variables then things will get messy.

Edit: I'd also refrain from "using namespace std;", as you might end up with naming collisions.

edited Apr 14 '17 at 13:46

answered Apr 14 '17 at 13:28

Cl9

46
3

I tried copying only this segment and pasting it into a different file and it worked. However when I run it in my other program, which includes other code, it initiates an endless loop. – Dev Apr 14 '17 at 13:40
Could you update your question with a snippet that has the issue then? If you use something like 'continue' at any point, you could be jumping back to the top. – Cl9 Apr 14 '17 at 13:43

Attila · Answer 2 · 2017-04-14T13:28:31.027

0

Most likely you don't need the & operator before input5 on the scanf line, because the things scanf expects for %s fields are already pointers/arrays. Although I don't see how input5 is declared, so I'm not sure if this is the (only) problem. You should have included that in the code snippet also.

EDIT: Just a note: It's not particularly elegant to mix C-style (scanf) and C++ style (cout) IO. You wouldn't have this problem with cin.

edited Apr 14 '17 at 13:28

answered Apr 14 '17 at 13:24

Attila

1,445
1
11
21

This is a comment, not an answer. Answers should be definitive; this says *may be the answer, but I'm not sure* and suggests that the poster should have provided more details. – Ken White Apr 14 '17 at 13:26
input5 is declared as a char. – Dev Apr 14 '17 at 13:29
@Dev: You keep saying it's a `char`, but it's really a `char[]`. They are different. – AndyG Apr 14 '17 at 13:49
@Dev: The declaration of `input5` should be in your question. Stop adding details in comments and instead [edit] your post. – Ken White Apr 14 '17 at 14:04

score 0 · Answer 3 · answered Apr 14 '17 at 13:43

Obviously, the loop does not terminate because the string comparison does not return equality. And this must be because input5 does not contain the typed input (and for the same reason, the else clause is executed whatever the input).

As input5 is only modified by the scanf call, this must be the root of the evil. A simple debugging session would have revealed it immediately.

The reason is simple: you must pass scanf the address of the buffer, but you are actually passing the address of the address, and overwriting the value of the variable input5 (for which we don't have the declaration but can infer char* or const char*).

In a 32 bits environment, this could cause a crash by overwriting the stack. Under 64 bits, you'll need more typing to obtain it.

Beginner here - what does it mean to pass the address of the scanf to the buffer? How do I do it? — Dev, Apr 14 '17 at 13:49

Endless do while loop in C++ code?

3 Answers3