views:

206

answers:

4

I have been attempting to write a program that will determine if a number is prime or not. I have based it off of the Sieve of Eratosthenes. Anyway, my program works for small numbers (15485863 works), but if I use large numbers (ex. 17485863) I receive a segmentation fault. I am using unsigned long longs and do not think I have surpassed their maximum value. I just don't see what I have done wrong. Thank you in advance for any assistance!

#include <iostream>
#include <limits>

using namespace std;

bool soe (unsigned long long);

int main (void)
{
 unsigned long long x = 17485863;
 bool q = soe(x);

 cout << x << " is ";
 if(q)
  cout << "prime." << endl;
 else
  cout << "not prime." << endl;

 return 0;
    }

    bool soe(unsigned long long input)
    {
 unsigned long long arrayLength = input%2 + input/2;
 unsigned long long index = 2;
 unsigned long long greatestMult = 0;
 bool array[arrayLength];

 array[0] = true; //ignore true values in the array
 array[1] = true;
 do{
  array[index] = false;
 }while(++index < arrayLength);

 index = 2;

 do
 {
  if(input%index != 0)
  {
   greatestMult = input/index;
   while(index*greatestMult > arrayLength)
    greatestMult--;
   do
   {
    array[index*greatestMult] = true;
   }while(--greatestMult > 0);

   do
   {
    if(!array[index])
     break;
   }while(++index < arrayLength);

  }
  else
  {
   cout << endl << input << " is divisble by " << index << endl;
   return false;
  }
 }while(index < arrayLength);

 return true;
    }
+1  A: 

On Line 24 you have: bool array[arrayLength]; You cannot declare an array on the stack like this. The program is crashing on line 29. You need to declare this on the heap using new/delete;

Something to this effect (I may have a leak or two in there, but you get the idea);

 //Beginning on Line 28
 bool *array = new bool[arrayLength];

 array[0] = true; //ignore true values in the array
 array[1] = true;
 do{
  array[index] = false;
 }while(++index < arrayLength);

 index = 2;

 do
 {
  if(input%index != 0)
  {
   greatestMult = input/index;
   while(index*greatestMult > arrayLength)
    greatestMult--;
   do
   {
    array[index*greatestMult] = true;
   }while(--greatestMult > 0);

   do
   {
    if(!array[index])
     break;
   }while(++index < arrayLength);

  }
  else
  {
   cout << endl << input << " is divisble by " << index << endl;
   delete [] array;
   return false;
  }
 }while(index < arrayLength);

 delete [] array;
 return true;
    }

Output

g++ -g test.cpp
gdb ./a.out
...clipped...
(gdb) run 
Starting program: /Users/nextraztus/a.out 
Reading symbols for shared libraries ++. done

17485863 is divisble by 3
17485863 is not prime.

Program exited normally.
(gdb) 
Nextraztus
+3  A: 

Please note that neither long long nor using variables to dimension automatic arrays are part of C++ - they are extensions provided by gcc and should not be used if portability is an issue.

To address your problem, dimensioning an array like this:

 bool array[arrayLength];

will cause a stack overflow (and thus a seg fault) if the arrayLength value is too large. Use a std::vector instead, but be aware that memory is not an infinite resource.

anon
+3  A: 

Stack overflow.

Amit Kumar
A: 

It is possible for index*greatestMult to be equal to arrayLength, so you can overwrite the last element past the array end.

Also allocating large arrays on the stack like that can cause a problem depending on the operating system. Some systems will expand the stack that much, others will not be able to.

ergosys