-2
#include <stdio.h>
int factorial(int value){
    if(value == 0){
        value * factorial(value - 1);
    }
}
int main()
{
    int value = 0;
    printf("The factorial of %d of %d",value, factorial(value));
}

I got an output of

The factorial of 0 is -1

Can anyone explain why I got -1 ???

I tried to watch a few videos on this but I didn't get the answer

Explain the logic of this if possible?

Thank you

Codebegginer
  • 5
  • 5
  • 5
    1. You're not returning the value from your function. 2. In the line `if(value == 0){` you probably meant `if(value > 0){` – Anton Curmanschii May 15 '21 at 19:43
  • 1
    Compile with the `-Wall` option. You'll get two warnings [that you should fix]: "value computed is not used" and "control reaches end of non-void function" – Craig Estey May 15 '21 at 19:46
  • @AntonCurmanschii well I know that I made a mistake in it its actually not even a complete factorial program I wondered what will happen if I didn't return anything – Codebegginer May 15 '21 at 20:24
  • You are invoking UB if you do not return anything. https://stackoverflow.com/a/58424449/5550963 –  May 15 '21 at 21:02
  • Another UB is your function is overflowing if you pass `17` into it, because you are returning value larger than `MAX_INT` (maximum signed integer). –  May 15 '21 at 21:04
  • @Codebegginer see my answer, I know its a bit complex for beginner but what it boils down is that your processor is using same "memory" for `value` and `return`. –  May 15 '21 at 21:40

2 Answers2

5

There are a couple problems with factorial: It's never returning anything, and it's recursing when 0 is passed to it (that should be the basis case and should return 1). Try this:

int factorial(int value) {
    if (value == 0) {
        return 1;
    }
    return value * factorial(value - 1);
}
Tom Karzes
  • 22,815
  • 2
  • 22
  • 41
  • 1
    Should also add to the if statement " || value ==1". – George May 15 '21 at 19:53
  • 1
    @George It's not needed for correctness, but it would speed up small cases. I wanted to keep to the original intent as closely as possible. – Tom Karzes May 15 '21 at 19:58
  • I guess that's fair. I imagined he was eventually going to change value to be something other than 0 (I hope). I will remember this for answering future questions! – George May 15 '21 at 20:00
  • Well I wanted wrote the program with a few mistakes I didn't return the value and || value ==1 must have been used and I checked this with a few other values then it gives the same value in return I guessed that if the function doesn't have a program in to run then it will simply give the same value which it received in return ex : value = 10 // in my program output is 10 // giving the same value – Codebegginer May 15 '21 at 20:29
  • I'm just trying to know how it works by making it kind of wrong sorry if wasted the time I didn't intend the question to help me with the factorial program I intended it to know how it works – Codebegginer May 15 '21 at 20:32
  • @Codebegginer read this: https://stackoverflow.com/a/58424449/5550963 –  May 15 '21 at 20:42
  • Hey its OPs problem as well, but use at least `long`, if you pass `17` you are invoking UB. –  May 15 '21 at 20:59
1

Can anyone explain why I got -1 ???

This is undefined behavior, but possible answer is that compiler is using same register

    .text
    .globl  factorial
    .type   factorial, @function
factorial:
.LFB0:
    .cfi_startproc
    pushq   %rbp    #   
    .cfi_def_cfa_offset 16
    .cfi_offset 6, -16 
    movq    %rsp, %rbp  #,  
    .cfi_def_cfa_register 6
    subq    $16, %rsp   #,
    movl    %edi, -4(%rbp)  # value, value
# fact.c:14:    if(value == 0) value *factorial (value - 1); so if value != 0 execute these 2 lines
    cmpl    $0, -4(%rbp)    #, value
    jne .L2 #, jump to L2 if not equal to 0
# fact.c:14:    if(value == 0)  value *fact (value -1); execute next 4 lines if value == 0
    movl    -4(%rbp), %eax  # value, tmp90
    subl    $1, %eax    #, _1
    movl    %eax, %edi  # _1,
    call    factorial    #   
.L2:
# fact.c:17: }
    nop 
    leave
    .cfi_def_cfa 7, 8
    ret 
    .cfi_endproc

So register eax is used for return value, thus if you look this assembly snippet:

    movl    -4(%rbp), %eax  # value, tmp90
    subl    $1, %eax    #, _1
    movl    %eax, %edi  # _1,
    call    factorial    #

You see that value have been moved into %eax, than 1 is subtracted and factorial is called again in which case its value is not 0 thus it will exit.

EAX: The accumulator. This register typically stores return values from functions.

from this page.