Why is this program running when input is 1?

Question

I wrote a C program to use recursive functions to find the factorial of a number and the code is given below:

//program to find factorial of a number using a recursive function
#include <stdio.h>
#include <conio.h>
#include <stdlib.h>
int factorial(int a);
int main()
{
    int num, fact;
    printf("Enter a number: ");
    scanf("%d", &num);
    if(num<=0)
    {
        printf("The number cannot be zero/negative.");
        exit(1);
    }
    fact=factorial(num);
    printf("The factorial of the entered number is: %d", fact);
    getch();
    return 0;
}
int factorial(int a)
{
    while(a>1)
    return a*factorial(--a);
}

In the above code in 'while' loop if the input 'a'>1 then it should execute, but when i compile the program (using Dev C++ v.5.4.2) and give the input as 1, I get the factorial as 1. The program is the solution to the required problem, but i need to know what is actually happening in background...

Thank you in advance

When you call `factorial(1);`, it doesn't return anything and the access to `fact` leads to *undefined behaviour*. — P.P, Nov 23 '15 at 15:52
You already have recursion. The `while` is unnecessary and, as described, will cause problems. — Karoly Horvath, Nov 23 '15 at 15:52
@BlueMoon What do you mean by 'undefined behaviour' ? But when I enter 1, I get the factorial as 1 which is the correct answer, Why is it so... — Chandrapal, Nov 23 '15 at 15:54
Too bad... In `factorial()`, if `a>1` it is undefined behavior for using decrement, and if `a<=1` it is undefined behavior for not executing `return statement... — MikeCAT, Nov 23 '15 at 15:55
I mean [this](https://en.wikipedia.org/wiki/Undefined_behavior). Seemingly getting expected results is also a part of it. — P.P, Nov 23 '15 at 15:55
@KarolyHorvath The while loop is necessary, without it the loop goes to infinite looping and stops the program... — Chandrapal, Nov 23 '15 at 15:56
@Chan Undefined behavior = you have a run-time bug which makes your program behave outside what is specified/guaranteed by the C language specification. Meaning that anything can happen. [See this](http://stackoverflow.com/questions/2397984/undefined-unspecified-and-implementation-defined-behavior). — Lundin, Nov 23 '15 at 15:56
Thank you @MikeCAT, if and while will work in the same way here I think, when i was just changing the original code to find out how the factorial(1) works, I just changed that to while. Thank you anyways — Chandrapal, Nov 23 '15 at 15:59
So it turns out that the statement `while(a>1) return a*factorial(--a);` contains two different cases of undefined behavior, while at the same time using recursion... May I humbly suggest that you rewrite this program using a loop instead? — Lundin, Nov 23 '15 at 16:06
May I know what are the two different cases of undefined behaviour.. @Lundin — Chandrapal, Nov 23 '15 at 16:18
@Chan One is the missing return statement. The other is more tricky to understand. In plain English: never modify a variable in the same expression where you are using that variable for something else, which is unrelated - the compiler might get confused. In C language gibberish: you access `a` twice inside the same expression with no sequence points in between and the expression contains "an unsequenced side affect". [See this](http://stackoverflow.com/questions/949433/why-are-these-constructs-using-undefined-behavior) for details. — Lundin, Nov 23 '15 at 16:23

ameyCU · Accepted Answer · 2015-11-23T15:58:00.500

3

int factorial(int a)
{
    while(a>1)
    return a*factorial(--a);
}

This function does not return anything if the condition in while loop is not true. Thus , invoking undefined behaviour (Maybe giving you correct output).

You should handle that case -

int factorial(int a)
{
   if(a==1)return 1;
   //while(a>1)               // you use recursion then why using loop 
   return a*factorial(a-1);
}

edited Nov 23 '15 at 15:58

answered Nov 23 '15 at 15:52

ameyCU

16,489
2
26
41

1

I suggest `return a*factorial(a-1);` since there is no sequence point between `a` and `factorial(--a)`. – R Sahu Nov 23 '15 at 15:56
@RSahu Yes,made that change thanks !! :-) . Due to `a` was getting decrementated , output coming was also wrong . – ameyCU Nov 23 '15 at 15:58

Lundin · Answer 2 · 2015-11-23T16:01:21.730

2

In the case where a <= 1 your function does not have a return statement, even though it expects one. Which means you have a bug which will invoke undefined behavior: anything can happen, including: "seems to work ok", "weird output", the program crashes etc.

A half-decent compiler would warn you for this. For example GCC with warnings enabled:

warning: control reaches end of non-void function [-Wreturn-type]|

edited Nov 23 '15 at 16:01

answered Nov 23 '15 at 15:53

Lundin

195,001
40
254
396

1

That warning is about `a*factorial(--a);`, not about the missing `return`. – R Sahu Nov 23 '15 at 15:58
@RSahu Ooh right, there's multiple undefined behavior on that single line. That's impressive! Turns out I just pasted the wrong error though. Thank you, it has been fixed :) – Lundin Nov 23 '15 at 16:01

score 0 · Answer 3 · answered Nov 23 '15 at 15:53

This is undefined behavior.

N1256 6.9.1 Function definitions

12 If the } that terminates a function is reached, and the value of the function call is used by the caller, the behavior is undefined.

To know what is actually happening in this specific situration, have your compiler output assembly code and read it, or use debugger suce as OllyDbg.

score 0 · Answer 4 · answered Nov 23 '15 at 16:06

Your compiler seems to allow omission of return value. What happens is that your "factorial" function will not execute the while() loop, and after the while loop there is no return statement, so the return value will be undefined.

int factorial(int a)
{
    while (a>1) // a is 1 so this will be false, and the loop will not execute
        return a*factorial(--a);
    return 1; // this value will be returned
}

Also, I suggest you review this code (looks like a school exercise to me). Currently, although it may work correctly, the code differs from how an experienced programmer would implement it.

Why is this program running when input is 1?

4 Answers4