1
#include < stdio.h > 
#include < process.h > 
rec();
main() {
    int a, fact;
    char question, n, y;

    do {
        printf("\nEnter any number ");
        scanf("%d", & a);
        fact = rec(a);
        printf("Factorial value = %d\n", fact);
        printf("do you want to exit.....(y/n):");
        scanf("%s", & question);
    }
    while (question == n);
    exit(0);
}
rec(int x) {
    int f;
    if (x == 1) return 1;
    else f = x * rec(x - 1);
    return f;
}

In this program I want to get factorial of the entered number, which I get. But I also want the user to say whether to exit or get the factorial of another number, which I can't do. It asks user but when I enter "n" it exits.

Where is the error?

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Rahul Subedi
  • 131
  • 1
  • 3
  • 12
  • 1
    You need to use `strcmp` to compare string values. – RageD Feb 27 '13 at 01:15
  • 1
    Note that the compiler is not obliged to accept `#include < stdio.h >`; it should be written `#include ` without spaces inside the angle brackets. Also, you should learn to write C99-compliant code. That means explicit return types are needed and prototype declarations: `extern int rec(int x);` and `int main(void)`. You can use `return 0;` at the end of `main()` to remind you it returns an `int`. – Jonathan Leffler Feb 27 '13 at 01:16
  • thanks . you are great :) – Rahul Subedi Feb 27 '13 at 01:45
  • @Kay Why not? It's a great way to input words... – autistic Feb 27 '13 at 01:45
  • @Kay Imagine if our ancestors had told us "Don't use fire, because you might get burnt"... – autistic Feb 27 '13 at 01:48
  • 1
    @modifiablelvalue: the unstated reason may be 'avoid [stack overflow](http://stackoverflow.com)'. Using `"%19s"` to read into a `char answer[20];` variable avoids overflowing the buffer (but note the 'different by one' lengths, and that `scanf()` won't accept the length as an input parameter, unlike `printf()`). Otherwise, there is no reason not to use it other than the general issue of 'usability of the UI' when you do. There's no reason not to use `fgets()` and `sscanf()` when you want to split stuff up into words. – Jonathan Leffler Feb 27 '13 at 01:51
  • ... then why not say "Don't use `scanf` without first reading the manual carefully"? Simply stating "Don't use `scanf("%s", ...)`" is a lot like "Don't use fire", or "Don't use a car", or "Don't use a compound slide saw". – autistic Feb 27 '13 at 01:59
  • @Kay I feel resentful that you would assume my understanding in this regards. I can read, you know? I am the one who suggested "Don't use `scanf` without first reading the manual carefully". What does that imply about how I operate? Would you assume someone who reads alot doesn't know about shell code, format string vulnerabilities, buffer overflows (and the more lethal underflows), nopslides, etc? – autistic Feb 27 '13 at 02:33
  • I noticed @Kay removed both of his/her comments, without mentioning anything. I can only assume this was for the malicious purpose of rendering this discussion irrational. I'll say no more than: Where does my name come from? What do exploits have to do with C? If you can use the first to justify your answer for the second, then you'll understand why your mention of exploits was irrelevant here. – autistic Feb 27 '13 at 15:27

2 Answers2

5

You want

while (question == 'n');

Or

char question, n = 'n', y = 'y';

Though I find the 2nd version a little redundant.

Either way you need to change

scanf("%s"

to

scanf("%c"

To correctly read in a single char and not a string. Thanks RageD

Karthik T
  • 31,456
  • 5
  • 68
  • 87
  • He may also want to change `%s` to `%c` in his `scanf` statement. +1 – RageD Feb 27 '13 at 01:15
  • Note that reading a single character with `%c` is normally going to get the newline after the number, not the response on the next line (indeed, it will appear to continue without waiting for any more input, and since `'\n' != 'n'`, the loop will exit every time). You can finesse that with `scanf(" %c", &question);` which skips white space before reading a character, including a newline. – Jonathan Leffler Feb 27 '13 at 01:31
1

One problem is the combination of:

char question, n, y;

scanf("%s", &question);

You are using %s to read a null-terminated string into a single character. Even if you hit 'y' and return, you'll be overwriting beyond the end of the variable. This is not good. (The good news is that "%s" skips over white space, including the newline after the number).

You either need to use "%c" in the format:

char question;
scanf(" %c", &question);  // NB: The leading space is important!

or you need to use a string format and a string variable (and no &):

char question[10];
scanf("%9s", question);

If you use an array, you need to consider whether to use strcmp(), or whether to compare the first character from the input:

while (strcmp(question, "n") == 0);
while (question[0] == 'n');

You probably got told by the compiler that you'd not declared variable n so you added it. You probably need the loop to end with while (question == 'n');and then get rid of the (now) unused variablen(and the currently unused variabley`).

Note that if you use omit the space in the " %c" format string:

scanf("%c", &question);

then it will normally get the newline after the number, which won't be 'n', so your loop will exit every time, apparently without waiting for you to enter anything. You can finesse that with scanf(" %c", &question); which skips white space before reading a character.

You should test that scanf() received the input you expected each time you use it. The correct test for single item inputs is:

if (scanf(" %c", &question) != 1)
    ...input failed...

If you need to distinguish between EOF and conversion failure, you can capture the return from scanf():

int rc;
if ((rc = scanf(" %c", &question)) != 1)
    ...rc == EOF on EOF; rc == 0 on 'conversion failure'...
    ...a single character input can't easily fail...
    ...but if someone types 'a' instead of '9' when you're looking for a number...

Getting I/O right using scanf() is distressingly hard. Many experienced programmers simply don't use it; it is too hard to get right. Instead, we use fgets() or POSIX getline() to read a line of data, and then use sscanf() to parse it. There are many advantages to this, but a primary one is that the newline has been eaten so you don't run into problems with the variable question not containing the answer you expect.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278