Why does this recursive factorial function hit segmentation fault?

This is the recursive function I'm using to calculate the factorial of an int. It compiles fine but when I run it, the program hits segmentation fault.

The function receives an int taken from the user. The function call is fact(&n).

GDB is not helping because when I "step" into the fact function it hits segmentation fault on the first pass through it.

Also the code is very simple and it is adjusted to the pseudo code from the walkthroughs/shorts.

Where is this code going so bad?

int fact (int* b)

int main (void)
{
int a;
do
{
a = GetInt();
}
while (a < 1);

fact(&a);
printf("Factorial: %i\n", a);
}

int fact (int* b)
{
if (*b == 1)
{
return *b;
}
else
{
*b = *b * (*b - 1);
fact(b);
}
return *b;
}

The fact() function is a recursive function that takes a pointer to an int as input. However, the internal recursive call, return *a * fact(a - 1); is not passing a valid pointer to an int. It's clear that you want to pass the next smaller integer, but you are actually passing the contents stored at the pointer minus 1, instead of a pointer address. This eventually leads to an attempt to access unauthorized or invalid memory.

If you need to pass an address pointer, then you need to be consistent. If not, it would be much simpler to just pass and return integers across the board instead of pointers.

Hmmm, looks like I wasn't precise enough. My bad. In your recursive call, because of the way the function parameters are designed, you need to pass an address of the memory containing the number to be multiplied. Instead, you are passing the address of a minus 1, meaning that you are subtracting 1 from the address, not the value stored at the address. While writing this as an exercise to understand pointers(a good thing), it has problems. You have two options - either correct the recursive call or change it to eliminate all the pointers.

If you want to keep all the pointers, the recursive call needs to be changed. First, you need to subtract 1 from the value stored in a somehow, probably with an intermediate variable. Next, you need to execute the recursive call using that new value, while preserving a for the return part of the statement. You can't simply subtract 1 from an address because this would access a different area of memory than what you want.

Also, this means that when you initially call the function from main, you have to pass it the address of the data, not the actual data.

Or, you could simplify everything. Get rid of all the pointers and just pass by copy:

int fact (int a)
{
if (a == 1)
{
return a;
}
else
{
return a * fact(a - 1);
}
}

This also means that you would update your calling code with something like this:

int n = 4;
int result = fact(n);  // this would store 24 in result.

It completely eliminates the pointers and corrects the recursion address problem.

In your comment, you said that " I'm trying to get this function to get passed the value from an int variable in Main,..." If you are passing an int variable, then you need to do the latter. If you pass it an int var when it is expecting a pointer to an int var , then it is going to fail.

The new function in your edit creates an infinite loop. Next, it isn't doing anything with any of the return values. The remaining issue is how a function is called. Here is the simplified method of doing this without the complexity of pointers, mallocs, etc. You need to master this first.

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

int fact (int b);

int main (void)
{
int a;
do
{
a = GetInt();
}
while (a < 1);

int result = fact(a);
printf("Factorial: %i\n", result);
}

int fact (int b)
{
if (b == 1)
{
return b;
}
else
{
return b * fact(b - 1);
}
}

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)

• I'm sorry, I simply do not understand your answer. I understand at least in a basic way how pointers work. I'm trying to get this function to get passed the value from an int variable in Main, then modify it with recursion and then return the modified value. I'm still not seeing where things went wrong, again, maybe somebody should explain this to me using puppets or something. Oct 12 '16 at 13:10
• "but you are actually passing the contents stored at the pointer minus 1, instead of a pointer address" Is this not what I'm supposed to do? If I make the function non-recursive with the following code: *a = *a * (*a - 1) it actually works, but whenever I want to call the function again it doesn't. Oct 12 '16 at 21:28
• See edited answer. Oct 12 '16 at 23:06
• Ok, I tried some of the approaches you suggested to the best of my ability. I understand that subtracting 1 from an address seg faults, and I understand why. However, I seem to be unable to find a way to make the function recursive without hitting seg fault. For example if I do this: int fact (int* b) { if (*b == 1) { return *b; } else { *b = *b * (*b - 1); } return *b; } with 5 as input the function executes once and successfully returns 20 to Main. The moment I add fact(b) it seg faults after the first pass through. Oct 13 '16 at 22:54
• Ummm..... I already wrote it. See the answer. Oct 13 '16 at 22:57