While Loop Terminating Unexpectedly (C/C++)

Question

I simply have no idea what is going wrong. I have a while loop (code block 1) that terminates immediately after one iteration of the loop. I have even commented out the entire switch statement so only the printf(...) and scanf(...) remain, and the loop seems to terminate.

The loop:

int main(int argc, char *argv[]) {
if(argc != 2) {
    printf("Usage requires hostname\n");
    return 1;
}

TCPClient client;
char str[1];
unsigned char buf[BUFLEN];
bool run = true;

printf("The commands in use are:\n"
    "h - help\n"
    "r - read ENCs (incremental)\n"
    "x - exit\n");

while(run) {
    printf("Enter a command: ");
    scanf("%s", str);

    switch(str[0]) {
      case 'h':
        printf("The commands in use are:\n"
            "h - help\n"
            "r - read ENCs (incremental)\n"
            "x - exit\n");
        break;

      case 'r':
        client.recvData(argv[1], buf, BUFLEN);

        for(int i = 0; i < BUFLEN / 4; i++) {
            if(i % 3 == 0)
                printf("\n");

            printf("Encoder %d: %d\t", i, (int)((((buf[4 * i] << 24) | (buf[4 * i + 1] << 16)) | (buf[4 * i + 2] << 8)) | (buf[4 * i + 3])));
        }
        printf("\n");
        break;

      case 'x':
        run = false;
        break;

      default:
        printf("Unrecognized command\n");
        break;
    }
}

return 0;
}

At one point, I added

if(run)
    printf("run = true");

after the while loop to see if it was breaking due to the control bool (run) being false. The result was no output, indicating that somehow, run became false. This happens no matter what the input is, or even if the switch statement is gone, in which case nothing should make run become false.

What's really strange is that in another file, I have a similar loop, except it actually works:

void *control(void *arg) {
DAC dac(5);
char str[1];
int chan = -1;
uint16_t code = -1;

while(run) {
    printf("Enter a command: ");
    scanf("%s", str);

    switch(str[0]) {
      case 'h':
        printf("The commands in use are:\n"
            "h - help\n"
            "s - set DACs\n"
            "r - read ENCs (incremental)\n"
            "t - read ENCs (cumulative)\n"
            "q - Toggle the quadrature waveform\n"
            "x - exit\n");
        break;

      case 's':
        chan = code = -1;

        printf("Enter the DAC channel: ");
        scanf("%d", &chan);

        printf("Enter the value: ");
        scanf("%hu", &code);

        pthread_mutex_lock(&lock);
        dac.setDAC(chan, code);
        pthread_mutex_unlock(&lock);
        break;

      case 'r':
        pthread_mutex_lock(&lock);
        for(int i = 0; i < BUFLEN; i++) {
            if(i % 3 == 0)
                printf("\n");
            printf("Encoder %d: %d\t", i, incBuf[i]);
            incBuf[i] = 0;
        }
        printf("\n");

        pthread_mutex_unlock(&lock);
        break;

      case 't':
        pthread_mutex_lock(&lock);
        for(int i = 0; i < BUFLEN; i++) {
            if(i % 3 == 0)
                printf("\n");
            printf("Encoder %d: %ld\t", i, cumBuf[i]);
        }
        printf("\n");

        pthread_mutex_unlock(&lock);
        break;

      case 'q':
        runSignal = !runSignal;
        if(runSignal)
            printf("Quadrature waveform enabled\n");
        else
            printf("Quadrature waveform disabled\n");

        break;

      case 'x':
        run = false;
        break;

      default:
        printf("Unrecognized command\n");
        break;
    }
}

dac.reset();
pthread_exit(NULL);
}

I'm pretty lost with what's happening...

This is a really obvious thing, but I have to ask just in case - you are actually providing an argument to the program aren't you? — scl, Jul 30 '13 at 06:43
Yes, I do. Either way, the program would simply quit if I didn't. The check at the top ensures that. — Hari, Jul 30 '13 at 06:49

Grijesh Chauhan · Accepted Answer · 2013-07-30T06:59:51.417

3

In while-loop scanf("%s", str); is wrong because the declaration of str[] is char str[1]; it can store only one char (as size is one). But scanf() stores at-least two char (including \0). Suppose if you inputs only c its stores as "c" that is consists of two char first c and second is \0 that causes a buffer-overflow -- Undefined behavior according to C standards.

edited Jul 30 '13 at 06:59

answered Jul 30 '13 at 06:43

Grijesh Chauhan

57,103
20
141
208

Interesting. Why does it work in the latter case (the working code), but not the former case? Is the buffer simply overflowing into something else in the latter case? In any case, changing the declaration to `char str[2]` fixed it. Thank you. – Hari Jul 30 '13 at 06:50
@HariGanti Is undefined behavior it doesn't guaranty that it will work always, actually in worst case it will work. – Grijesh Chauhan Jul 30 '13 at 06:53
Fixing it to str[2] is not sufficient. A rogue user can simply input a string with two characters and the same error will occur. – Martin Richtarsky Jul 30 '13 at 07:09
@MartinRichtarsky yes! actually `scanf()` its self is not safe! – Grijesh Chauhan Jul 30 '13 at 07:10
@MartinRichtarsky, while that's true, I am the only one who uses this, and it's largely a test program. Also, your argument would be true for any string length. – Hari Jul 30 '13 at 15:24
@HariGanti yes if input is large, we can't test, preferable way is `fgets()`, read: http://stackoverflow.com/questions/17294809/reading-a-line-using-scanf-not-good/17294869#17294869 – Grijesh Chauhan Jul 30 '13 at 15:30

Martin Richtarsky · Answer 2 · 2013-07-30T06:52:51.453

1

This is likely a stack buffer overflow of str overflowing into run during scanf. Comment out scanf to verify this.

edited Jul 30 '13 at 06:52

answered Jul 30 '13 at 06:44

Martin Richtarsky

619
7
18

While Loop Terminating Unexpectedly (C/C++)

2 Answers2