Code loops when character entered in C

Question

This is mostly out of curiosity to why this is happening as it doesn't matter in my case. If I type in an invalid number it properly goes to the repeat label and asks me to enter a number again, but if i enter in a character like 'f' it will loop endlessly and not stop. Why is this?

The array and all variables here are of type int.

    repeat: 
    printf("Enter number of available space, you are %c: ", userXO);
    scanf("%d", user);

    switch (*user)
    {
        case 1: if (spaces[0][0] == 49){ spaces[0][0] = userXO;}else goto repeat; break;
        case 2: if (spaces[0][1] == 50){ spaces[0][1] = userXO;}else goto repeat; break;
        case 3: if (spaces[0][2] == 51){ spaces[0][2] = userXO;}else goto repeat; break;
        case 4: if (spaces[1][0] == 52){ spaces[1][0] = userXO;}else goto repeat; break;
        case 5: if (spaces[1][1] == 53){ spaces[1][1] = userXO;}else goto repeat; break;
        case 6: if (spaces[1][2] == 54){ spaces[1][2] = userXO;}else goto repeat; break;
        case 7: if (spaces[2][0] == 55){ spaces[2][0] = userXO;}else goto repeat; break;
        case 8: if (spaces[2][1] == 56){ spaces[2][1] = userXO;}else goto repeat; break;
        case 9: if (spaces[2][2] == 57){ spaces[2][2] = userXO;}else goto repeat; break;
        default: goto repeat; break;
}

Whatever happened to indenting? That code is practically unreadable. — sapi, Apr 21 '13 at 01:14
Why would you use [goto](http://stackoverflow.com/questions/3517726/what-is-wrong-with-using-goto)? Why?! WHYYYY?? — timss, Apr 21 '13 at 01:15
@sapi it is indented. If you are talking about the if statement, it's not very complicated and I didn't indent so it could fit on the screen. — 0x41414141, Apr 21 '13 at 01:16
@timss to save time. I know it's bad practice. I'm sure those downvotes are probably from that. — 0x41414141, Apr 21 '13 at 01:17
you enter a character and you read it as int? of course it is causing problem. What you want is read the 'user' as string. I don't see why your goto save time. — marcadian, Apr 21 '13 at 01:23
@marcadian Aren't characters just of int type? 49 is 1 for example — 0x41414141, Apr 21 '13 at 01:24
Seems what you want is validating the input. Are you sure you the (invalid) input is always one character? — marcadian, Apr 21 '13 at 01:27
Always check the status from `scanf()`. It is busy returning `0` which indicates that it cannot interpret the input as a number, but that you have not reached EOF yet. So, the test must be: `if (scanf("%d", user) == 1) { ...do your stuff... } else { ...handle error or EOF... }`. Note that one of the few times it is legitimate to use `feof()` is in the 'handle error or EOF' code. In this case, both `feof()` and `ferror()` will report OK, so you know that you're not at EOF and have not got an I/O error, so the data must be faulty. — Jonathan Leffler, Apr 21 '13 at 01:42

score 5 · Accepted Answer · answered Apr 21 '13 at 01:21

5

scanf("%d", user); tries to read a number, finds a char(f), leaves it in the buffer and ends. The loop then loops around and executes scanf("%d", user); again. And again...

answered Apr 21 '13 at 01:21

Is the what a "buffer overflow" is? – 0x41414141 Apr 21 '13 at 01:22
2

@bh3244 no, a "buffer overflow" is when you try to access elements past the end of an array. – Kevin Apr 21 '13 at 01:25

score 2 · Answer 2 · answered Apr 21 '13 at 01:56

Here is how I would have written what you did:

int rc, user;
char buf[100];

for (;;)  // repeat until explicitly broken out of
{ 
    printf ("Enter number of available space; you are %c: ", userXO);
    if (!fgets (buf, sizeof buf, stdin))  /* end of file or i/o error? */
        break;

    rc = sscanf(buf, "%d", &user);
    if (rc != 1)   /* other than one parsed input item is an error */
    {
        printf ("invalid number; try again\n");
        continue;
    }

    /*
     * this switch has the odd property of potentially
     * doing all 9 cases for case 1,  8 cases for case 2, etc.
     * Maybe explicit breaks for success are needed?
     */
    switch (user)
    {
    case 1: if (spaces[0][0] == 49) spaces[0][0] = userX0; else continue;
    case 2: if (spaces[0][1] == 50) spaces[0][1] = userX0; else continue;
    case 3: if (spaces[0][2] == 51) spaces[0][2] = userX0; else continue;
    case 4: if (spaces[1][0] == 52) spaces[1][0] = userX0; else continue;
    case 5: if (spaces[1][1] == 53) spaces[1][1] = userX0; else continue;
    case 6: if (spaces[1][2] == 54) spaces[1][2] = userX0; else continue;
    case 7: if (spaces[2][0] == 55) spaces[2][0] = userX0; else continue;
    case 8: if (spaces[2][1] == 56) spaces[2][1] = userX0; else continue;
    case 9: if (spaces[2][2] == 57) spaces[2][2] = userX0; else continue;
    default: continue;
    }
    break;  /* if valid case(s) taken, exits loop */
}

As you can see, there is no need for a label or a goto. Also the code is more compact.

Code loops when character entered in C

2 Answers2