Summation of nth prime

Question

I'm writing a program that finds the summation of prime numbers. It must use redirected input. I've written it so that it finds the largest number for the input then uses that as the nth prime. It then uses the nth prime to set the size of the array. It works up until I try to printf the sum. I can't figure out why I'm getting a seg fault there of all place.I think I've allocated the arrays correctly with malloc. why would the fault happen at printf on not when I'm using my arrays? Any suggestions on my code are also welcome.

EDIT used a test input of 2000 form 1 to 2000 and it worked but the full test file of 10000 form 1 to 10000 crashes still looking into why. I'm guessing I didnt allocate enough space

EDIT My problem was in my sieve I didnt take the sqrt(nthprime) so it found more primes then the array could hold

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


int nprime (int max);
void sieve_sum ( int *primes, int nthprime, int tests,int *input);

int main(void)
{
    int i=0;
    int max=0; //largest input
    int tests; //number of tests
    int nthprime; //estimated nth prime
    int *primes; //array of primes
    int *input; // numbers to put in to P(n), where p(n) is the summation of primes

    scanf("%d",&tests); //gets number of tests
    input = malloc(sizeof(int)*tests);

    //test values
    for(i=0; i<=tests-1; i++)
        scanf("%d",&input[i]);

    //finds max test value
    i=0;
    for (i = 0; i < tests; i++ )
    {
        if ( input[i] > max-1 )
            max = input[i];
    }

    // calls nprime places value in n
    nthprime = nprime(max);
    primes = malloc(sizeof(int)*nthprime);

    // calls sieve_sum
    sieve_sum( primes, nthprime, tests, input);

    //free memory
    free(input);
    free(primes);
    return 0;
}

//finds Primes and their sum
void sieve_sum ( int *primes, int nthprime, int tests,int *input)
{
    int i;
    int j;

    //fills in arrays with 1's
    for(i=2; i<=nthprime; i++)
        primes[i] = 1;

    //replaces non primes with 0's
    i=0;
    for(i=2; i<=sqrt(nthprime); i++)
    {
        if(primes[i] == 1)
        {
            for(j=i; (i*j)<=(nthprime); j++)
                   primes[(i*j)] = 0;
        }
    }

    //rewrites array with only primes
    j=1;
    i=0;
    for(i=2; i<=nthprime; i++)
    {
        if(primes[i] == 1)
        {
            primes[j] = i;
            j++;
        }
    }

    //sums
    i=0;
    for ( i=1; i<=tests; i++ )
    {
        int sum=0;//sum of primes

        j=0;
        for(j=1; j<=input[i-1]; j++)
        {
                sum = primes[j] + sum;
        }

        printf("%d\n", sum );
    }
    return 0;
}

//finds the Nth number prime
int nprime (int max)
{
    //aproximization of pi(n) (the nth prime) times 2 ensures correct allocation of memory
    max = ceil( max*((log (max)) + log ((log (max)))))*2;
    return (max);
}

Example input file:

20
1
2
3
4
5
6
7
8
9
10
10
9
8
7
6
5
4
3
2
1

Example output should be:

2 
5 
10 
17 
28 
41 
58 
77 
100
129
129
100
77
58
41
28
17
10
5
2

Answer 1

OK, so I'm going to have a shot at this without saying too much because it looks like it might be a homework problem.

My best guess is that a lot of people have shied away from even looking at your code because it's a bit too messy for their level of patience. It's not irredeemable, but you would probably get a better response if it were considerably cleaner.

So, first, a few critical comments on your code to help you clean it up: it is insufficiently well-commented to make your intentions clear at any level, including what the overall purpose of the program is; it is inconsistently indented and spaced in an unconventional manner; and your choice of variable names leaves something to be desired, which is exacerbated by the absence of comments on variable declarations.

You should compile this code with something like (assuming your source file is called sumprimes.c):

gcc -std=c99 -pedantic -Wall -Wextra -o sumprimes sumprimes.c -lm

Have a look at the warnings this produces, and it will alert you to a few, admittedly fairly minor, problems.

The major immediate issue that I can see by inspection is that your program will certainly segfault because the storage that you have allocated with malloc() is too small by a factor of sizeof(int), which you have omitted.

A static error-checker like splint would help you detect some further issues; there's no need to slavishly follow all of its recommendations, though: once you understand them all, you can decide which ones to follow.

A few other remarks:

• “Magic numbers”, such as 100 in your code, are considered very bad form. As a rule of thumb, the only number that should ever appear in code literally is 0 (zero), and then only sometimes. Your 100 would be much better represented as something named (like a const int or, more traditionally, a #define) to give an indication of its meaning.
• It is unconventional to “outdent” variable declarations in the manner done in your code
• If a function is advertised to return a value, you should always check it for errors, e.g. make sure that the return value of malloc() is not NULL, check that the return value of scanf() (if you use it) is the expected value, etc.
• As a general matter of style, it is usually considered good practice in C to declare variables one per line with a short explanatory comment. There are exceptions, but it's a reasonable rule-of-thumb.
• For any kind of input, scanf() is a poor choice because it alters the state of stdin in ways that are difficult to predict unless the input is exactly as expected, which you can never depend on. If you want to read in an integer, it is much better to read what's available on stdin with fgets() into a buffer, and then use strtol(), as you can do much more effective error-checking and reporting that way.
• It is not recommended to cast the return of malloc() any more.

Hope this helps.