recursive function error: "stack overflow"

Question

I wrote this function that supposed to find smallest positive integer that is divisible by every number 1 through 20. I get "stack overflow error", even though, when I test for numbers 1 through 10, it works just fine. here is my code:

#include<iostream>
#include<cstdlib>

using namespace std;

// function prototype
int smallPos(int k, int m,int n);

int main(){
    printf("the smallest positive number that is divisible by 1 through 20 is %d ", smallPos(1,1,13));

}

int smallPos(int k, int n, int m){
    int div=0;
    for(int i = n;i<=m;i++) {
        if (k%i==0) 
            div++;
    } 
    if (div==m) {
        return k;
    } else {
        k+=1;
        smallPos(k,n,m);
    }   
}

Why does it happen? The number shouldn't be that big anyway. Thank you!

Why is this even implemented as a recursive function, anyways? — André Caron
– André Caron, Commented Jul 12, 2011 at 2:49
This was a solution I could come up with. I tried to use nested loops but got confused. — GKED
– GKED, Commented Jul 12, 2011 at 2:56
You could save yourself some looping and recurse on the first fail: for(int i = n; i <= m; i++) if(k % i) return smallPos(k + 1, n, m); return k; but the stack overflow will still exist. — Chris Lutz
– Chris Lutz, Commented Jul 12, 2011 at 2:59

André Caron · Accepted Answer · 2011-07-12 03:05:28Z

5

The final condition (div == m) will never be true. For div to become equal to m, the number k should be divisible by all of the numbers in range [n,m).

Edit: I've reread the text in the printf() call to understand what the function does. You're looking for the first number divisible by all numbers in the range. If my calculations are correct, this number should be the product of all unique prime factors of the numbers in the range. For the range [1,13] (as per the function call, not the text), this number should be:

30030 = 1 * 2 * 3 * 5 * 7 * 9 * 11 * 13

Now, this means you are going to invoke the function recursively over 30,000 times, which is obviously way too many times for the size of stack you're using (defaults are relatively small). For a range this size, you should really consider writing an iterative function instead.

Edit: here's an iterative version that seems to work.

int smallPos ( int n, int m )
{
    int k = 0;
    while ( ++k )
    {
        int count = 0;
        for (int i = n; i<=m; i++)
        {
            if (k%i==0) {
                ++count;
            }
        }
        if (count == (m-n+1)) {
            return k;
        }
    }
    return k;
}

Indeed, the result for smallPos(1,10), the result is 2520. It seems my previous estimate was a lower bound, not a fixed result.

edited Jul 12, 2011 at 3:05

answered Jul 12, 2011 at 2:45

André Caron

45.5k12 gold badges72 silver badges125 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

GKED Over a year ago

That will be true for a large number, which I yet to figure out :). For example, the least positive integer that is divisible for all i 1:10 is 2520.

André Caron Over a year ago

Shouldn't it be 1890 (1 * 2 * 3 * 5 * 7 * 9)?

GKED Over a year ago

I think you are right, I should try to figure out the iterative way.

André Caron Over a year ago

In any case, you'll need to increase m because the smallest number divisible by all of [n,m] will necessarily be larger than m.

Kerrek SB · Accepted Answer · 2011-07-12 02:54:13Z

1

Your smallPos function incurs undefined behaviour since it is not returning a value in all execution paths. You may want to say return smallPos(k,n,m); in the last part (or just return smallPos(k + 1, n, m); in one go).

edited Jul 12, 2011 at 2:54

answered Jul 12, 2011 at 2:43

Kerrek SB

480k96 gold badges904 silver badges1.1k bronze badges

3 Comments

André Caron Over a year ago

While this is true, that's not what's causing the stack overflow.

Kerrek SB Over a year ago

@Andy: You're right, the OP's function never actually gets to return in the current version... anyway, my UB beats your logical reasoning any day ;-)

André Caron Over a year ago

hahaha! If you bet UB over logical reasoning, there's not reasoning with you and I'll just let you think you've won :-)

Collectives™ on Stack Overflow

recursive function error: "stack overflow"

2 Answers 2

4 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related