Segfault in Primes Program

[orwell84]

okay, so I wrote a program in Perl to find prime numbers and, when learning C, I decided to translate it into C. The thing is that...well...it doesn't work. No matter what I do, I get the same message when I say ./primes.c at the terminal: Segmentation fault. I know what this is, but I just can't put my finger on where it came from in my program. It would be much appreciated if anyone could figure this out for me, or help me to find it.

#include <stdio.h>

int main() {
	int maxprimes;
	int count = 1;
	int value = 1;
	int primes[maxprimes];
	int composite;
	int i = 1;
	int j;

	printf ("How many primes? ");
	scanf ("%d", &maxprimes);

	printf ("2 is prime\n");
	primes[0] = 2;
	while (count < maxprimes){
		composite = 0;
		value += 2;
		for (j = 1 ; j < maxprimes ; j++) {
			if (value % primes[j] == 0) {
				composite = 1;
				break;
				}
			}
		if (composite = 0){
			printf ("%d is prime", value);
			primes[j] = value;
			count += 1;
			j++;
			}
		}
	}

To explain the code a little, it's based on a Sieve of Eratosthenes algorithm, that's what the array is for. It's also probably the source of the segfault (the array, that is)

[csurfer]

C runs on some rules,you need to learn them before trying out something,and ya you cannot just translate a code to C and expect it to work.

int maxprimes;
int count = 1;
int value = 1;
int primes[maxprimes];
int composite;
int i = 1;
int j;
printf ("How many primes? ");
scanf ("%d", &maxprimes);

This is wrong. You are trying to take the value of maxprimes from the user and trying to allocate that much memory for the array primes. But as you can see compiler would have already allocated memory for that array and at that time

int primes[maxprimes];

doesn't have any meaning,so the segmentation fault.

What you can do is to take the value of maxprimes and then allocate memory for array,but in C

int maxprimes;
int count = 1;
int value = 1;
int composite;
int i = 1;
int j;
printf ("How many primes? ");
scanf ("%d", &maxprimes);
int primes[maxprimes];

something like this is not allowed.Because in C all data variable's should be declared and initialized before the first operational statement(say an assignment or a check.).Its memory also needs to be allocated priorly if you are allocating it statically.

Now coming to the solution : You need to use dynamic allocation of memory Do this :

//Within main function
int maxprimes;
int count = 1;
int value = 1;
int *primes;
int composite;
int i = 1;
int j;

printf ("How many primes? ");
scanf ("%d", &maxprimes);

//your solution is this statement
primes = (int *)malloc(maxprimes * sizeof(int));

And ya I haven't checked your algorithm "Sieve of Eratosthenes" hope you haven't done any mistake in it. ;) !!!

[orwell84]

Thanks for the solution, but now it won't compile. Doesn't int *primes have to be an array?

[siddhant3s]

Arrays and pointer are closely related in C. But first tell me what book are you using?
Anyways, one advice: your scripting languages like Python and Perl makes you feel that programming is fun, this is not the case with C. I am sure you will have to do quite low level things in C. Know that C is statically typed.

If you are just beginning to C, don't use the above said solution. It will tend to confuse you.
You must know that you must explicitly state the size of an array which should be a CONSTANT. Thats the most important luxury you miss when coming from perl. In C we have fixed sized array. ( we too have hacks for dynamically sized array but that would be covered later).
So, the solution would seem like this:

#include <stdio.h>
/*The maximum prime that the Program supports*/
#define MAXPRIMES 100
int main() {
	int max;
	int count = 1;
	int value = 1;
	/*note below, here I passed a constant value,100 as size*/
	int primes[MAXPRIMES];
	int composite;
	int i = 1;
	int j;
    /*I warned the user that maximum primes supported by this program*/
	printf ("How many primes?(Maximum %i)",MAXPRIMES);
	scanf ("%d", &max);
	
	
    /*I am too tired to check your progarm logic.
    As you are a programmer, SHould I trust you?
    Anyways, if you can make it run, get us back*/
	printf ("2 is prime\n");
	primes[0] = 2;
	while (count < maxprimes){
		composite = 0;
		value += 2;
		for (j = 1 ; j < maxprimes ; j++) {
			if (value % primes[j] == 0) {
				composite = 1;
				break;
				}
			}
		if (composite = 0){
			printf ("%d is prime", value);
			primes[j] = value;
			count += 1;
			j++;
			}
		}
	
	}

[orwell84]

I get a floating point exception now. I'll revisit this when I learn about memory allocation...by the way, I'm using the massive book called the Internet

[siddhant3s]

It is really when people say : `` I am using Internet as a book". Internet is no book. Internet can be as closely be thought as network of books. Nothing more.
A book is a book. Don't wander much here and there. Stick to one resource for learning. (though you may search as much as you like for a particular reference)

Another thing is that : When you say ``I have a error", do mention the line number where the compiler complaints.
Anyways, do have a look at :http://www.dreamincode.net/code/snippet528.htm

[zalezog]

If max is the number of primes ,you originally want then why are you looping to maxprimes (or is it MAXPRIMES)
( @siddhant3s :

if you can make it run, get us back.

)

//while (count < maxprimes)
//shouldn't it be   
(count < max)

But even after warning the user that the maximum limit is MAXPRIMES ,no checks are done whether actually the value entered
by scanf is within range or not.

How about this line , something's missing,no?

if (composite = 0)

[siddhant3s]

@zalezog
The code below line 21 has not been touched by me.

>If max is the number of primes ,you originally want then why are you looping to
>maxprimes
Because the OP had maxprimes as his variable. I changed it to max for simplicity. It was the OP's responsibility to convert each occurrence of maxprime to max.

>shouldn't it be .....
Yes it should, but thats OPs job, why are you getting dirty in a dirty code.

Rest of all your suggestion are for (the benefit of ) OP

[orwell84]

Thanks for the other error catch. Like I said, I'm going to wait to come back to this problem when I know about memory allocation. If I have to limit the amount of primes that I can find, why make it in the first place?
By the way, about having no checks for it, I was getting to that. Once the code compiled, I would put the checks in there.
Also, I didn't give the compiler message and line # because, as I've mentioned twice already, I realize that I have a lot more to learn before attempting this again.

[orwell84]

Okay, this is solved now. Here's the finished code:

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

int main() {
	int maxprimes;
	int count = 1;
	int value = 1;
	int composite;
	int i = 1;
	int j;

	printf ("How many primes? ");
	scanf ("%d", &maxprimes);
	int *primes = malloc(maxprimes * sizeof(int));
	if(primes == NULL) {
		fprintf(stderr, "Out of memory, exiting\n");
		exit(1);
		}
	printf ("2 is prime\t[#1]\n");
	*primes = 2;
	while (count < maxprimes){
		composite = 0;
		value += 2;
		for (j = 0 ; j < count ; j++) {
			if (value % (*(primes + j)) == 0) {
				composite = 1;
				break;
				}
			}
		if (composite == 0){
			printf ("%d is prime\t[#%d]\n", value, (count+1));
			*(primes + count) = value;
			count += 1;
			}
		}
	}