1

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?

gvlasov
  • 18,638
  • 21
  • 74
  • 110
Ducoodi
  • 21
  • 5
  • 1
    Are you sure about the line: scanf("%c"),&height_in_inches; ? I think it should be scanf("%c",&height_in_inches); – B3rn475 Jun 07 '15 at 21:25
  • you can check the scanf function here(http://www.cplusplus.com/reference/cstdio/scanf/) for more examples how to use it right – CIsForCookies Jun 07 '15 at 21:29
  • 2
    WRONG: `scanf("%c"),&height_in_inches;`. BETTER: `scanf("%c", &height_in_inches);`. That's assuming you want to input a binary, 8-bit integer value (e.g. for "10", enter on your keyboard. If you want a *STRING*, (like the characters "10"); I'd recommend: BEST: `int height_in_inches; scanf ("%d", &height_in_inches);` You don't really need "atoi()" in either case... – paulsm4 Jun 07 '15 at 21:35
  • @paulsm4 I'm with you on wanting to use %d. I did try using that, but when it didn't work I wasn't sure which part to change, so that was one of them. – Ducoodi Jun 07 '15 at 21:45
  • Actually, Jonathan Leffler's (new!) answer is the best of all. TO SUMMARIZE: 1) your original crash was because of incorrect "scanf()" syntax. 2) If you want to input a string, then it's also wrong to use a 1-byte "char": you should declare an n-byte "char[]" array. 3) But frankly, it makes most sense to let "scanf" do the work for you, and declare "height_in_inches" the type you ultimately need (e.g. "float" or "double"). STRONG SUGGESTION: look again at Jonathan Leffler's reply. – paulsm4 Jun 07 '15 at 22:11
  • @paulsm4 Leffler's response might be the most appropriate, but the code was too advanced for me at this stage of my learning. Step by step. – Ducoodi Jun 07 '15 at 22:19
  • With all due respect... horsecrap ;) If something appears "too advanced" ... then it's probably a a good *OPPORTUNITY* ... to dig in and *LEARN*. – paulsm4 Jun 08 '15 at 02:56
  • @paulsm4 Which is precisely what I'm doing, in my own time. Thanks for the help. – Ducoodi Jun 08 '15 at 06:10
  • The posted code does not even come close to compiling. Strongly suggest , when compiling, to enable all warnings (for gcc, at a minimum, use '-Wall -Wextra -pedantic' ) The output from the compiler will be several warnings, Warnings are important, the compiler knows the language much better than us humans. Fix the warnings. Suggest, when using a system function call, for instance scanf(), read the man page for that function. Then you will know the syntax and expected returned value. Use that man page info when writing the call to the system function. – user3629249 Jun 09 '15 at 13:31

2 Answers2

3

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.

Community
  • 1
  • 1
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • Your examples, especially the second one, are a bit ahead of me. I am truly a beginner. Although I am getting quite excited to learn about `if` and `else` from what I have seen so far. – Ducoodi Jun 07 '15 at 22:02
  • 3
    I'm sorry to have gone beyond what you've learned. I've not got the book, but would have expected basic coverage of `if`, `while` and `for` before doing much in the way of input — evidently, I was wrong. Come back in a week or two; it will make a lot more sense then. A key point to take away is that input operations (in particular) can fail, and the success/failure of input operations should be checked before you use the result. It's a good idea to echo what you read. If you think you entered 10 but the computer thinks you entered 31415332132, it gives you a good hint that something's wrong. – Jonathan Leffler Jun 07 '15 at 22:07
  • I see, that does make sense. The book is very much "this does this, and that does that", which at my point of discovery is okay, but it has caused problems when I've messed around with the examples. Still, K&R was just too much so I'm happy for now. – Ducoodi Jun 07 '15 at 22:13
  • a LOT of stack space can be saved by 1) noticing that a 'int' is only 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' – user3629249 Jun 09 '15 at 13:38
  • 2
    @user3629249: see extensive addition to the answer addressing your observations. – Jonathan Leffler Jun 09 '15 at 14:24
0

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);
B3rn475
  • 1,057
  • 5
  • 14
  • I'm not following you entirely. I'm not familiar with casts or points yet, but your code change helped, although I had to put it in as: `char height_in_inches[20];` for it to work. – Ducoodi Jun 07 '15 at 21:40
  • 1
    @Ducoodi wants to avoid `gets()` but then you recommend using `scanf()` with `%s`? Use at least `%19s` to prevent a potential buffer overflow. – cremno Jun 07 '15 at 22:07
  • 1
    It would be sensible to provide compilable code. `char [20] height_in_inches;` isn't going to work with most C compilers (it should be `char height_in_inches[20];`, of course). – Jonathan Leffler Jun 07 '15 at 22:09
  • @cremno, why `%19s` and not `%20s`? What happens to that 1? – Ducoodi Jun 07 '15 at 22:16
  • 2
    The normal convention is to specify the full buffer size, but the standard I/O library predates the widespread use of that convention, and it chose to go with the length of string that can be stored. In a `char buffer[20];` array, you can store a string of up to 19 characters plus the null terminating byte. `strlen()` doesn't count the null terminating byte either. It's a minor inconsistency designed to make life harder for beginners. Well, not really. It is just a legacy of development over more than 30 years (unless you argue the design was made in the late 70s and frozen in the late 80s). – Jonathan Leffler Jun 07 '15 at 22:20
  • 1
    Sorry my fault the char[] is in C# not C. – B3rn475 Jun 08 '15 at 07:20
  • 1
    @Jonathan Leffler Minor disagree about the string `20` vs. `19` rational. The number `19` in `"%19s"` represents the maximum (non-white-space) characters to scan - it is the scan width. This is consistent with `"%3d"`, `"%7f"` and all specifiers to limit the scan width. The width parameter does not represent the space needed to store the object. It is just that with strings, the size needed is width + 1. OTOH, it would have been good had original `scanf()` provided easier dynamic string size limits. – chux - Reinstate Monica Jun 09 '15 at 15:47
  • 1
    @chux: That's another, probably more valid, way of looking at it. Nevertheless, there is a mismatch (diff-by-one) between the size used to define a string array and the number that should appear in a `scanf()` conversion specification, which can cause confusion. And [I 100% agree](http://stackoverflow.com/questions/1621394/) that it should be possible to tell `scanf()` et al the size of a string more simply. [The `scanf_s()` approach](http://stackoverflow.com/questions/372980/) is one way; it works, but isn't entirely obvious. – Jonathan Leffler Jun 09 '15 at 16:12