Pregunta

Okay, so this function I created uses the Sieve of Eratosthenes algorithm to compute all the primes <= n. This function stores the prime numbers and the count of primes in the parameters.

When the function exits, primes should be pointing to a chunk of dynamically allocated memory that holds all the primes <= num. *count will have the count of primes.

Here is my function getPrimes:

void getPrimes(int num, int* count, int** array){
    (*count) = (num - 1);
    int sieve[num-1], primenums = 0, index, fillnum, multiple;

    //Fills the array with the numbers up to the user's ending number, num.
    for(index = 0, fillnum = 2; fillnum <= num; index++, fillnum++){
        sieve[index] = fillnum;
    }

    /* Starts crossing out non prime numbers starting with 2 because 1 
       is not a prime. It then deletes all of those multiples and 
       moves on to the next number that isnt crossed out, which is a prime. */
    for (; primenums < sqrt(num); primenums++)  //Walks through the array.
    {
        //Checks if that number is NULL which means it's crossed out
        if (sieve[primenums] != 0) {
            //If it is not crossed out it starts deleting its multiples.
            for (multiple = (sieve[primenums]); 
                 multiple < num; 
                 multiple += sieve[primenums]) {  
                //Crossing multiples out 
                //and decrements count to move to next number
                sieve[multiple + primenums] = 0;
                --(*count);
            }
        }
    }
    int k;
    for(k=0; k < num; k++)
        printf("%d \n", sieve[k]);

    printf("%d \n", *count);
    array = malloc(sizeof(int) * (num + 1));
    assert(array);
    (*array) = sieve;
}

Now, here is the intended output and my output. As you can see, something is wrong within my getPrimes function but I am unsure as to what.

Intended Output:

There are 8 prime numbers less than or equal to 19

2  3  5  7  11  13  17  19  

My Output: 

2 
3 
0 
5 
0 
7 
0 
0 
0 
11 
0 
13 
0 
0 
0 
17 
0 
19 
0 
0 

Here are 3 problems that people have pointed out to me so far:

  1. Wrong delete process if (sieve[multiple]) { array sieve index has bias
  2. (*array) = sieve; leaks the just-malloced memory, and lets *array point to a local variable that ceases to exist when the function returns - you'll get a dangling pointer.
  3. if(sieve[i] != NULL) should use 0, not NULL, you're not having an array of pointers.

However, I'm not too sure how to fix the dangling pointer/memory issue that has been spotted out for me. Besides that, I was wondering if there was anything else within my code that has errors as I am not too sure why my numbers in my output are adding the 0's...don't worry about the different output style, just the extra numbers. Thanks if you can help me out with this!

¿Fue útil?

Solución

void getPrimes(int num, int* count, int** array){
    (*count) = (num - 1);
    int sieve[num-1], primenums = 0, index, fillnum, multiple;

You're declaring an array of num-1 elements, for the numbers from 2 through num. That's okay.

    //Fills the array with the numbers up to the user's ending number, num.
    for(index = 0, fillnum = 2; fillnum <= num; index++, fillnum++){
        sieve[index] = fillnum;
    }

You're filling each slot with the number it associated with, also okay.

     /* Starts crossing out non prime numbers starting with 2 because 1 is not a prime.
        It then deletes all of those multiples and 
        moves on to the next number that isnt crossed out, which is a prime. */
     for (; primenums < sqrt(num); primenums++) //Walks through the array.

You're stopping roughly at the square root, that's good.

        {
            if (sieve[primenums] != 0){ //Checks if that number is NULL which means it's crossed out

                   for (multiple = (sieve[primenums]); multiple < num; multiple += sieve[primenums])
                      //If it is not crossed out it starts deleting its multiples.
                   {  //Crossing multiples out and decrements count to move to next number
                            sieve[multiple + primenums] = 0;

You're having a problem here. You only stop the loop when multiple >= num, but you're writing to the index multiple + primenums, and that is often past the end of the array. For example, with num == 19, and primenums == 1 (which crosses out the multiples of 3), the last write is to index 18 + 1, but the last valid index is num - 2 = 17.

The indexing bias of point 1 has been fixed. But

                            --(*count);

You're unconditionally decrementing *count here, in the earlier code you only decremented it when sieve[multiple] was not already crossed out. That is the correct way. I suggest

for(multiple = primenums + sieve[primenums]; multiple < num - 1; multiple += sieve[primenums]) {
    if (sieve[multiple]) {
         sieve[multiple] = 0;
         --(*count);
    }
}

to have it a bit simpler.

                   }
            }
        }
        int k;
        for(k=0; k < num; k++)
            printf("%d \n", sieve[k]);

You're printing the contents of sieve regardless of whether it is 0 or not, and you also print out sieve[num - 1], which does not exist. Make it

for(k = 0; k < num-1; ++k) {
    if (sieve[k]) {
        printf("%d\n", sieve[k]);
    }
}

to print only the primes.

            printf("%d \n", *count);
        array = malloc(sizeof(int) * (num + 1));
         assert(array);
         (*array) = sieve;
}

Replace the (*array) = sieve with

int i = 0;
for(k = 0; k < num-1; ++k) {
    if (sieve[k]) {
        (*array)[i] = sieve[k];
        ++i;
    }
}

to write only the primes to *array. Also, you need not allocate (num + 1)*sizeof(int), but only *count * sizeof(int) for that.

Otros consejos

The numbers you are printing are of the sieve, so all numbers that are not prime are set to 0. Try printing as follows

for (k = 0; k < num; k++)
if (sieve[k] != 0)
{
        printf(" %d\n", sieve[k]);
}
printf("\n");

Also you should not return the local array sieve through the array parameter, as it is located on the stack and it will no longer be available on return of the function.

Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top