How to take ints from user using fgets in C?

Question

I'm a beginner at C. I'm trying to write a program that computes the volume based on user's input of 3 integers using fgets(), and I'm struggling to understand why my code is not working.

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

int volumn(int a, int b, int c);

int main(int argc, char* argv[]){
    char* height, width, depth;
    fgets(&height, 10, stdin);
    fgets(&width, 10, stdin);
    fgets(&depth, 10, stdin);

    printf("\nThe volumn is %d\n", volumn(atoi(&height), atoi(&width), atoi(&depth)));

    return 0;
}

int volumn(int a, int b, int c){
    return a * b * c;
}

EDIT: I'm getting the following errors/warnings when I run the code above:

goodbyeworld.c:8:11: warning: incompatible pointer types passing 'char **' to
      parameter of type 'char *'; remove & [-Wincompatible-pointer-types]
    fgets(&height, 10, stdin);
          ^~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/usr/include/stdio.h:238:30: note: 
      passing argument to parameter here
char    *fgets(char * __restrict, int, FILE *);
                                ^
goodbyeworld.c:12:48: warning: incompatible pointer types passing 'char **' to
      parameter of type 'const char *'; remove & [-Wincompatible-pointer-types]
    printf("\nThe volumn is %d\n", volumn(atoi(&height), atoi(&width), a...
                                               ^~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/usr/include/stdlib.h:132:23: note: 
      passing argument to parameter here
int      atoi(const char *);
                          ^
2 warnings generated.

Solution 1

First of all, a definition like

 char* height, width, depth;

make height a pointer to char and the rest two as chars.

Secondly (not much relevant here, but in general, important), You did not allocate memory to the pointers you want to use (if at all).

If you have a fixed input length decided as 10, you can simply make all the three variables as array ans use the names directly, like

#define VAL 10
char height[VAL] = {0};
char width[VAL] = {0};
char depth[VAL] = {0};

and then

fgets(height, 10, stdin);

finally, consider using strtol() over atoi() for better error handling.

Solution 2

Simple solution of your problem.

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

int volumn(int a, int b, int c){
    return a*b*c;
}

int main(){

    char height[10];
    char width[10];
    char depth[10];

    printf("Please enter size of object:\n");
    fgets(height, 10, stdin);
    fgets(width, 10, stdin);
    fgets(depth, 10, stdin);

    int valheight = atoi(height);
    int valwidth = atoi(width);
    int valdepth = atoi(depth);

    printf("\nThe volumn is %i\n", volumn(valheight, valwidth, valdepth));

    return 0;
}

Comments

[Anders]

Hello and welcome to Stack Overflow! Could you be a bit more precise about how it's not working? Is it compiling? Do you get any error messages? If it runs, what happends when it runs? Please edit your question.

[user3629249]

Always, when compiling, enable all the warnings (for gcc, at a minimum use: -Wall -Wextra -pedantic) then fix the warnings. With the posted code you would see two warnings: 1) unused variable 'argc' 2) unused variable 'argv[]' along with the warnings you are already seeing.

[Admin]

I'd consider the missing allocation very relevant here, too

[Sourav Ghosh]

@FelixPalmen as OP's passing &height, the whole thing is wrong, anyway. :)

[Admin]

Indeed, but just fixing that issue, the segfault is next ;)

[Sourav Ghosh]

@FelixPalmen that's why I switched to arrays. :)

[Admin]

I never found scanf() of any use for real-world interactive input (and I assume that is what OP wants to do here)

[Coffee'd Up Hacker]

@FelixPalmen I fail to see how fgets is anymore "interactive" than scanf

[Admin]

using fgets(), you can react on unexpected input properly, something happening quite often in the interactive case.

[Coffee'd Up Hacker]

Don't both only return once a newline/EOF is encountered? If so, I still don't see how fgets is any better. Get the input, and do whatever you want to validate it after. With scanf you can at least specify what kind of data you're expecting, making validation much simpler.

[Admin]

Maybe simpler in some cases, but much less flexible, because it won't give you access to the original input. I'd always go with some other solution, e.g. strtol() like suggested in the other answer.

[Coffee'd Up Hacker]

Just use a %s then. Besides, conversions add unnecessary execution time, and converting something like "I'd like it to be 10" with strtol wouldn't give you what you'd expect anyway, while the %d with scanf allows it to fail and prompt again.

[johnwj]

How do I use strtol() to convert height to an int? I tried atoi(height), and it did not work.

[Jongware]

@tbee: how in the world could a simple atoi(height) not work??

[johnwj]

@Jongware I was running atoi(&height). Oops. atoi(height) does work.

[Admin]

This makes even less sense now. %s IS a conversion, of course it takes time, and it's utterly pointless when you want nothing other than that string.

[user3629249]

when calling scanf(), always check the returned value (not the parameter value) to assure the operation was successful. When using the format specifier '%s', always include a length modifier, so the user cannot overrun the input buffer.

[user3629249]

@Jongware, an atoi() can fail when the string pointed to by the parameter has no leading numeric value(s) for instance "abcedf" or " 123"

[Coffee'd Up Hacker]

@FelixPalmen My point is %s gives you more or less the same behavior as fgets, minus the forced newline. And no, %s is not a conversion, it's more of a type check, scanf fails if the input doesn't match the specifiers. With fgets, you have to get the string, which is one action, and then you have to convert it, which is another action, which may or may not throw an error because the input may or may not be a value convertible to a long.

[Coffee'd Up Hacker]

@user3629249 I had hoped that the use of %10s was implied by my use of %10d in my answer.

[Admin]

For the term conversion, see scanf(3). And no, an operation is not more efficient, just because you use one big library functions with tons of bells and whistles instead of 2 much simpler ones that even give you a lot more control. I rest my case, scanf() is a bad idea, at least for interactive input.

[Coffee'd Up Hacker]

@FelixPalmen So your argument is semantics. My point is fgets requires at least two actions, one of which is an explicit type conversion which the programmer has to take separate care to check against errors, meaning even more work and more code, more slow and more bloat. While scanf, with or without a type conversion (because it doesn't always do a conversion), does all of this simultaneously, simplifying everything, without sacrificing anything. You've failed to show how fgets is any better for "interactivity", or generally, better in any other way.

[Coffee'd Up Hacker]

@FelixPalmen All of this is moot anything, as OP has their answer. They seem fine with an fgets and strtol mix.