-1

My answer to problem PRIME1 please explain me where am I wrong. I am receiving a segmentation error.

here it is:

    #include<cstdlib>
    #include<iostream>

using namespace std;

int main(int argc, char** argv) {
int t=0,i=0,m=0,n=0;
cin>>t;
while(t--&&t<=10)
{
    cin>>m>>n;
    if(m>=1&&n-m<=100000)
    {
    int prime[n];
    for(i=0;i<n;i++)
        prime[i]=1;
    for (int i=2; i*i<=n; i++)
    { 
    if (prime[i] == true)
    {
        for (int j=i*2; j<=n; j += i)
            prime[j] = false;
    }
    }

    for (int k=m+1; k<n; k++)
        if (prime[k])
            cout <<k<<endl;
    }
}
return 0;
}
MikeCAT
  • 73,922
  • 11
  • 45
  • 70
ANiK3T
  • 47
  • 1
  • 8
  • 2
    `int prime[n];` may be too big to fit into the stack. It should be something like `char* prime = new char[n];`. Don't forget to `delete[]` it after using it. (I won't post this as answer because I didn't check this by submitting) – MikeCAT Aug 16 '16 at 15:16
  • `bool prime` may take up less space. `std::vector` would take up much less space, but with a performance hit that's not so helpful on SPOJ. – user4581301 Aug 16 '16 at 15:19
  • Not sure I get why this is downvoted. Perhaps I need schooling...¯\\_(ツ)_/¯ – tinonetic Aug 16 '16 at 16:49
  • You are using VLA (`int prime[n];` where `n` isn't a constant expression) which are not standard C++ and probably (gcc does it) allocated in the stack. Follow the suggestions by @MikeCAT or @user4581301 instead. – Bob__ Aug 18 '16 at 16:50

1 Answers1

0
for (int j=i*2; j<=n; j += i)
prime[j] = false;

this is what causes the bug, prime is an array of size n, meaning the index of its last member is n-1, this for statement tries to access prime[n] at its last iteration(j<=n, meaning n is still valid) to fix this change the condition to:

for(int j=i*2; j<n; ++j)

edit: perhaps the segmentation fault occurs due to bad logic of the code or poor formatting, after formatting the code, it compiles fine and does not cause a segmentation fault (tried with input {1,2,3} and {10,10,10} and a few more combinations):

#include <cstdlib>
#include <iostream>

using namespace std;

int main(int argc, char** argv) 
{
    int t=0,i=0,m=0,n=0;
    cin>>t;

    while(t--&&t<=10)
    {
        cin>>m;
        cin>>n;
        if(m>=1&&n-m<=100000)
        {
            int prime[n];

            for(i=0;i<n;i++)
            {
                prime[i]=1;
            }

            for (int i=2; i*i<=n; i++)
            { 
                if (prime[i] == true)
                {
                    for (int j=i*2; j<=n; j += i)
                    {
                        cout << "poopy\n";
                        prime[j] = false;
                    }
                }
            }

        for (int k=m+1; k<n; k++)
        {
            if (prime[k])
            {
                cout <<k<<endl;
            }
        }
        }
    }
    return 0;
}
monkeyStix
  • 620
  • 5
  • 10
  • i tried the code on a ubuntu 64bit system and it "works" (no segmentation fault, but i'm not sure about the logic), edit added – monkeyStix Aug 18 '16 at 15:35
  • logic is fine. i tried the code on ideone. everything goes well, but a strange segmentation error in SPOj :| – ANiK3T Aug 20 '16 at 09:24