-2

I would like to write a program which can find all prime numbers between two numbers in t test cases. But my program had crashed when I run it. Please, could anyone help me?

My code:

   #include <stdio.h>
#include <malloc.h>
#include <math.h>

void print(int a,int b)
{
    int *p,i;
    int x;
    p = (int *) malloc (sizeof(int)*(b-a));
    for(i=0;i<(b-a);i++) p[i]=a+i;
    for(i=0;i<(b-a)/2;i++)
    {
        if(p[i]!=0)
        {
            if(p[i]%i==0) p[i]=0;
        }

    }
    for(i=0;i<=(b-a);i++) if(p[i]!=0) printf("%d ",p[i]);
    free(p);
}

int main(void)
{
    int t,i,m,n;
    scanf("%d",&t);
    for(i=0;i<t;i++)
    {
        scanf("%d %d",&m,&n);
        print(m,n);
    }
    return 0;
}
Zagor
  • 3
  • 3
  • 3
    When you ran it under a debugger and stepped/breakpointed through, which line failed? – Martin James Feb 24 '15 at 17:27
  • 2
    [You should not cast the result of `malloc` in C.](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – Some programmer dude Feb 24 '15 at 17:28
  • Also, please run a debug-build of your program in a debugger to see *where* it crashes. Then examine the values of all involved variables to see if they are what you expect them to be (like `i` being a valid index). – Some programmer dude Feb 24 '15 at 17:31
  • Please read my comment (and the linked question/answers) again. – Some programmer dude Feb 24 '15 at 17:36
  • 1
    @Zagor Do not listen bad advices. You used the casting correctly. Now this line p = (int *) malloc (sizeof(int)*(b-a)); is self-descriptive and readres of the code know what is the type of p..:) – Vlad from Moscow Feb 24 '15 at 17:38
  • @Vlad from Moscow Then, where problem is? – Zagor Feb 24 '15 at 17:44
  • 2
    @VladfromMoscow That's your minority opinion; the link provided by Joachim explains what this cast is bad for resp. avoiding it is good for. – glglgl Feb 24 '15 at 17:51
  • @glglgl As it is well-known weak programmers form the majority. Even American president Obama said not so far in fact that he loves americans though there is the majority of idiots among americans. in USA:) – Vlad from Moscow Feb 24 '15 at 18:01
  • 1
    @VladfromMoscow Which of the arguments presented in the given Q/A are wrong in your opinion? – glglgl Feb 24 '15 at 18:03
  • @glglgl Totally all arguments are very siily. For example if you can do or can not do something in C/C++ it does not mean that you should do or do not do this.:) – Vlad from Moscow Feb 24 '15 at 18:06
  • 2
    @VladfromMoscow in a recent question, the naive code contained `int array[10000]; *array=(int)malloc(...);` and by casting the compiler would give NO warning of the error. What C++ does is NO argument for coding in C. – Weather Vane Feb 24 '15 at 18:07
  • Code allocates memory for `p[0]` to `p[b-a-1]`. In all 3 `for()` loops, code uses an index > `b-a-1`. 1) Fix code: do not access outside the allocated memory. 2) Re-think how much memory is needed. – chux - Reinstate Monica Feb 24 '15 at 18:08
  • @Weather Vane First of all this code can be valid and it shows the intention of the programmer. If the code is wrong then it indeed attract attention of a reader. – Vlad from Moscow Feb 24 '15 at 18:13
  • @VladfromMoscow no, it didn't show the coder's intention. I can't find the question, but he was trying to allocate memory for an `int` array that was already declared. – Weather Vane Feb 24 '15 at 18:20
  • @Weather Vane what you think, which lines should I correct in my code? – Zagor Feb 24 '15 at 18:22
  • @Weather Vane Take into account that there is used *array in the left side. So in fact it is a valid code. If some beginner do not understand what he is doing it does not mean that some construction is invalid.:) – Vlad from Moscow Feb 24 '15 at 18:25
  • Say code wants to find all the primes from `a:1,000,000,000` to `b:1,000,000,100`. Then each number would need to be tested if it was divisible by 2,3,5 ... up to `sqrt(n)` and not just `b-a:100` different numbers. So it makes sense to create an array on the order of `sqrt(b) + 1`or `(b+1)/2` depending on the following code. – chux - Reinstate Monica Feb 24 '15 at 18:26
  • @VladfromMoscow in that case, there was `warning C4047: '=' : 'int' differs in levels of indirection from 'void *'` which was hidden by the cast. – Weather Vane Feb 24 '15 at 18:28
  • @Zagor you havn't yet made the corrections advised. – Weather Vane Feb 24 '15 at 18:29
  • @@Weather Vane One more it can be a valid code that clearly shows the intention of the programmer. Why did you decide that such a code is invalid? – Vlad from Moscow Feb 24 '15 at 18:31
  • @vlad because it was crash-doom-bang. It *couldn't* be valid - the compiler would clearly say so! If you let it. – Weather Vane Feb 24 '15 at 18:32
  • @@Weather Vane Why did you decide that such a code results in crash-doom-bang? – Vlad from Moscow Feb 24 '15 at 18:36
  • @VladfromMoscow I didn't - the OP did, but using a cast hid the truth from him. – Weather Vane Feb 24 '15 at 18:44
  • I just edit my post - I use your suggestions to improve my code, but same thing happen – Zagor Feb 24 '15 at 19:01

2 Answers2

1

The problem is stumbling on allocating memory in a range, allocating one element too few (should have been malloc (sizeof(int)*(b-a+1));) and then not sticking to indexing the memory allocated. This could be so much simpler: no arrays needed - if a number has a divisor, there is no need to check any other divisors.

Sometimes it is easier to side-step the problems than struggle with them.

#include <stdio.h>
#include <math.h>

int prime(int n)
{
    int s, i;
    if (n == 1 || n == 2)
        return 1;
    if (n % 2 == 0)             // no even numbers
        return 0;
    s = (int)sqrt(n);           // limit the loop
    for (i=3; i<=s; i+=2)       // odd numbers only
        if (n % i == 0)
            return 0;
    return 1;
    }

void print(int a, int b)
{
    int n;
    for (n=a; n<=b; n++)
        if (prime (n))
            printf("%d ", n);
    printf("\n");
}

int main(void)
{
    int t, i, m, n;
    printf("Input number of ranges to test: ");
    scanf("%d", &t);
    for(i=0; i<t; i++)
    {
        printf("Input bottom and top of range: ");
        scanf("%d %d", &m, &n);
        print(m, n);
    }
    return 0;
}
Weather Vane
  • 33,872
  • 7
  • 36
  • 56
0

You messed up the termination of the for-loops. You allocated b-a bytes but you are iterating over b-a+1 items...

for(i=0;i<=(b-a);i++) p[i]=a+i;

needs to be a i<(b-a) or else you have a segfault (in both loops). Also as BLUEPIX pointed out:

for(i=0;i<b/2;i++)

needs to be i<(b-a)/2 for iterating over half the intervall.

p[i]%i

Division by zero in the first iteration i==0. After this the programm should terminate without an error.

hakononakani
  • 305
  • 2
  • 10
  • I just rewrite my code in way you mentioned, but same thing happen – Zagor Feb 24 '15 at 17:55
  • Sorry I forgot one thing to mention: the first iteration in the second for-loop is a divide by zero when i==0: p[i]%i obviously modulo 0 is pure evil – hakononakani Feb 24 '15 at 19:22