Replacing gets with scanf in simple equation crashes the program

Question

I'm working through C for Dummies. It has an example code for converting inches to cm (p.135, if you have the text) using gets and atoi.

Now, I wanted to try using scanf rather than the dreaded gets, and this is the best I could come up with (based on the code the author provided).

#include <stdio.h>
#include <stdlib.h>

int main()
{
    float height_in_cm;
    char height_in_inches;

    printf("Enter your height in inches: ");
    scanf("%c"),&height_in_inches;
    height_in_cm = atoi(height_in_inches)*2.54;
    printf("You are %.2f centimetres tall.\n",height_in_cm);
    return(0);
}

The program starts, but after input just crashes. Where am I going wrong?

Answer 1

Why spend time converting the answer when scanf() can do it for you?

#include <stdio.h>

int main(void)
{
    float height_in_inches;

    printf("Enter your height in inches: ");
    if (scanf("%f", &height_in_inches) == 1)
    {
        float height_in_cm = height_in_inches * 2.54;
        printf("You are %.2f inches or %.2f centimetres tall.\n",
               height_in_inches, height_in_cm);
    }
    else
        printf("I didn't understand what you said\n");
    return(0);
}

If you must read a string, then use fgets():

#include <stdio.h>
#include <stdlib.h>

int main(void)
{
    char line[4096];

    printf("Enter your height in inches: ");
    if (fgets(line, sizeof(line), stdin) != 0)
    {
        double height_in = strtod(line, 0);
        double height_cm = height_in * 2.54;
        printf("You are %.2f inches or %.2f centimetres tall.\n",
               height_in, height_cm);
    }
    return(0);
}

Note that both programs check that input occurred before using the results of the input. You can argue that the error checking for the call to strtod() is woefully lackadaisical; I'd agree. Note that I switched between float in the first fragment and double in the second; either can be made to work. I see no particular reason to limit the input to an integer value when the result will be a fraction. It is also often beneficial to echo the input as well as the output; if what you get echoed isn't what you think you entered, it is a good hint that something is horribly wrong and sends you searching in the right area of the code.

^{Note: there are a number of minor details brushed under the carpet, especially in the first example regarding float and double with scanf() and printf(). Given the comment below, they are not relevant to the OP at the moment.}

Minimal fixes to original code

Since the code above is more complex than the OP recognizes yet, here is a simpler set of fixes to the original code. The string input into an array (string) is the key point; that necessitated changes in the scanf() call too. By using a big buffer, we can assume that the user won't be able to overflow the input by typing at the terminal. It would not be OK for machine-driven inputs, though.

#include <stdio.h>
#include <stdlib.h>

int main(void)
{
    float height_in_cm;
    char height_in_inches[4096];    // Array, big enough to avoid most overflows

    printf("Enter your height in inches: ");
    // Missing error check on scanf() — too advanced as yet
    scanf("%s", height_in_inches);  // Format specifier, parentheses, ampersand
    height_in_cm = atoi(height_in_inches) * 2.54;
    printf("You are %s inches or %.2f centimetres tall.\n",
           height_in_inches, height_in_cm);
    return(0);
}

How long is a line of input?

user3629249 commented:

a LOT of stack space can be saved by:

1. noticing that an 'int' is only a max of 12 characters so the max length on the input buffer is 13 characters (to allow for the NUL string termination byte)
2. limit the scanf() to 12 characters. I.E. 'scanf( "%12s", myCharArray );
3. in C, the name of an array degrades to the address of the array, so not leading '&' needed on 'myCharArray'.

Point 3 is correct; if you use char myCharArray[13];, you do not use &myCharArray when calling scanf() et al; you use just myCharArray. A good compiler will point out the error of your ways if you misuse the &.

I have problems with points 1 and 2, though. A lot of trouble can be avoided by noting that if the user types 9876432109876543210 on the line, then using scanf() with %12s is not going to help very much in eliminating invalid inputs. It is going to leave 8 unread digits on the line, and what is read will still overflow a 32-bit integer. If you use the strtoX() family of functions on longer strings instead of atoi(), then they detect problems such as overflow, which neither scanf() with %d nor atoi() do. (This is one of the many points glossed over in the main answer.)

Also, on systems with megabytes, and usually gigabytes of main memory, 4 KiB on a stack is not a major problem. That said, I use 4 KiB in part for its shock value; but POSIX requires a minimum value of [LINE_MAX] of 2048.

If you are reading line-based inputs, which is usually a good way of doing input in command-line applications, then you want to make sure you read the whole line because futzing around with part lines is messy and makes error reporting hard. One major advantage of fgets() plus sscanf() processing is that you have a complete line which you can use in the error report, rather than what scanf() left after processing part of the line. You can also try scanning the string a different way if the first attempt fails; you can't do that with the direct file I/O functions in the scanf() family.

If you naïvely do not recognize that people type long lines when you intended them to type short lines, then you can get leftovers served as a new line — without further user interaction — when it is really the dregs of the previous line of input. For example, if you scan 12 digits out of 20, then the next input will get the remaining 8 digits without waiting for the user to type anything new, even though you prompted them for more input. (Also, beware of using fflush(stdin); it is at best system-specific whether it does anything useful.)

I used fgets() with a 4 KiB buffer. If you want to be safe against programs sending megabytes of JSON-encoded data on a single line, you need to use POSIX's getline() function to read the line. It allocates enough space for the whole line, unless it runs out of memory. For most student practice work, a 4 KiB buffer and fgets() is a reasonable surrogate. I'm willing to negotiate on the power of 2 used as long as the exponent is at least 8 — the value is at least 256. Limiting the line buffer to 80 characters, for instance, doesn't stop the user from entering more than 80 characters on a line. It just means that the extra characters are unlikely to be processed appropriately. Limiting the buffer to 13 characters doesn't buy you anything worthwhile, IMO, and 'saving stack space' is a premature optimization.

Answer 2

You are having troubles with a cast.

atoi accepts a const char* as input.

You are passing a char so it is implicitly casting the char to a point, bad bad thing.

As suggested by user3121023 change height_in_inches into a string.

char height_in_inches[20];

and read using %s

scanf("%s", height_in_inches);