Is this function to compute the factorial in C correct?

Question

I have been reading the book "Hacking - the art of exploitation" (2nd edition, No Starch Press) by Jon Erickson. On page 17, I have stumbled over some code in C of a function that should compute the factorial of a number. I have some programming skills in Python but I am a beginner in C. The code is:

int factorial(int x)
{
  int i;
  for(i=1; i < x; i++)
    x *= i;
  return x;
}

int a=5, b;
b = factorial(a);

It is written in the book that the variable b will contain 120, since the factorial function will be called with the argument of 5 and will return 120.

Is this correct? As far as I know, x is assigned a new number on every iteration, so the condition i < x is always true. This looks like an infinite for loop to me, but I may be wrong. I have checked the errata on the publisher's website, but I couldn't find information that cleared my question.

If I have made a mistake, can somebody explain me what is wrong? I attach an excerpt of the book for reference.

Answer 1

You're correct in your observation. The function as written will result in an infinite loop. This is because the condition i < x will always be true as x is being increased in each iteration of the loop (x *= i).

Based on your implementation, I will create another variable to hold the result, like so:

int factorial(int x)
{
   int i;
   int result = 1;
   for (i = 1; i <= x; i++)
      result *= i;
   return result;
}
int a=5, b;
b = factorial(a); // expected result = 120

Answer 2

You're right, this will not work to calculate the factorial. It's also not an infinite loop, since at some point you will get an integer overflow resulting in a negative value for x.

#include <stdio.h>

int factorial(int x) {
    int i;
    for (i = 1; i <= x; i++) {
        printf("%d\n", x);
        x *= i;
    }
    return x;
}

int main() {
    int a = 5, b;
    b = factorial(a);
    printf("%d\n", b);
    return 0;
}

Output:

5
5
10
30
120
600
3600
25200
201600
1814400
18144000
199584000
-1899959296

As you can see, the value of x wraps around and becomes negative when it exceeds the maximum positive value in integer can have in C (INT_MAX = 2147483647). Therefore the loop does end, but it definitely won't calculate the factorial.