segfault in fscanf and struct question

hey people, i get a segfault in this fscanf call according to gdb.

is there anything obviously wrong with it? im certain there is.

Code:

        while(fscanf(fp, "%ld", &input) != EOF)
	{
	        data_set->data * = input;
	    	data_set->items++;
	}

here is my full main function:
in here i want to put input into the set_t struct (code section below)

Code:

int
main(int argc, char **argv)
{
        long int  input;
	
	
	/* Parse Arguments */
	clo_t *iopts = parse_args(argc, argv);
	
	FILE *fp = fopen(iopts->input_file,"r");

	/*allocates memory for data_set*/
        set_t * data_set = set_create(500);

	data_set->items = 0;

	data_set->items++;

        while(fscanf(fp, "%ld", &input) != EOF)
	{
  /*not sure how to use this*/
	        data_set->data * = input;
              
	    	data_set->items++;
	}


	return (EXIT_SUCCESS);
}

this is the struct i want to send my variables to. however im not sure how to use int *data properly

Code:

typedef struct 
{
  int *data;
  int items;
  int n_max;
  int lock;
} set_t;

And here is set_create():

Code:

set_t *
set_create(int size)
{
  printf("size: %d\n", size);

  set_t * data_set = malloc(sizeof(set_t));
  
  data_set->data = malloc(size * sizeof(int));
  
  return(data_set);

}

everything before the fscanf works as it should im fairly certain.

thanks for any help.

data is of type int*. You are multiplying the pointer rather than the value its pointing at.

Also, it's not wise to multiply a long with an int and store the result in an int. You may quickly discover you exceeded MAX_INT.

ok i changed it to

Code:

data_set->data = input;

and still segfaulting,
is that what you mean? is it the value of data its pointing to now?

it dosnt get that far anyway, i have a printf statement as the first thing in my fscanf loop that dosnt print.

Code:

        while(fscanf(fp, "%ld", &input) != EOF)
	{
                printf("HEY\n");
	        data_set->data * = input;
	    	data_set->items++;
	}

i also dont understand how i can later look through my data values once they've been stored, do i set up a couple of set_t pointers that get the value of data up until items?

Originally Posted by AstralZecha

is that what you mean? is it the value of data its pointing to now?

Still very wrong -- the types match, but you have reassigned data_set->data that way meaning those 2000 bytes you allocated have now leaked, since data_set->data now points to input and not to the malloc'd memory.

Originally Posted by AstralZecha

Code:

        while(fscanf(fp, "%ld", &input) != EOF)
	{
  /*not sure how to use this*/
	        data_set->data * = input;

If data_set->data were a pointer to a single int, then

Code:

  /*not sure how to use this*/
	        *(data_set->data) * = input;

However, it's an array of 500 members, so in fact you have to refer to which member, eg:

Code:

data_set->data[0] *= input;

There's still a problem there, however, in so far as in data_set->data[0] has not been initialized to any value, and you are multiplying it.

Don't you need to index data? As it is 500 * int large? And why not read into data_set.data directly in the fscanf call?

You could probably do something similar to this:

Code:

for(i = 0; i < size; i++)
	fscanf(fp, "%d", &data_set.data[i]);

ok i now have:

Code:

        int i = 0;
	data_set->data[0] = 0;

        while(fscanf(fp, "%ld", &input) != EOF)
	{
	    printf("HEY2\n");
	    
	    
	    data_set->data[i]  = input;
	    	data_set->items++;

	    i++;
	}

which segfaults, i dont want to multiply anything.

i will try that subsonics

Originally Posted by AstralZecha

i dont want to multiply anything.

In that case you can take out "data_set->data[0] = 0"; this was only a problem if you were multiplying an unknown number (also, the point was to initialize each of them, not just the first one). But if you are setting each member to a definite value, that is initialization.

Nb, *= is the multiplication operator! "* =" is not quite the same thing, sorry for not pointing that out as an error (it is not any kind of valid syntax). I assumed it was meant to be multiplication.

i will try that subsonics

I would stick with the while loop and i++, I think Subsonics was just trying it illustrate the principle (that you have to iterate thru each member of data_set->data). However, you should put a limit in:

Code:

if (i>499) break;

thanks, will do.

i completely overlooked that i have a function for inserting new items set_insert() that i must use:

Code:

int
set_insert(set_t *set, int item, int *set_size)
{
  
  /*if data from file exceeds size*/
  if(data_set->items > 499)
  {
      /*realloc more space*/
  }
  
 /*set_size would refer to the malloc'd size i think, so how can i determine where i am in the array each time i call this function?*/
  data_set->data[?] = item;

}

revised main:

Code:

        while(fscanf(fp, "%d", &input) != EOF)
	{
	    
	    set_insert(data_set, input, 500);
 
	}

data_set undeclared in set_insert, but arent i using it as a parameter?

Originally Posted by AstralZecha

data_set undeclared in set_insert, but arent i using it as a parameter?

Yes, and data_set is a pointer so that will be fine. Just don't reassign it inside set_insert() -- you aren't, but that is important to remember and understand. Assigning to members of the struct is fine.

There are two ways to deal with this:

Code:

  data_set->data[?] = item;

The best way is to modify the prototype:

Code:

set_insert(set_t *set, int item, int *set_size, int element);

So you would submit "i" for the element num and then replace that ? with "element".

The other way, if you can't modify the prototype, is to use a static counter inside set_insert, but this limits the functionality in some probably undesirable ways. (You could also use a global counter, but that is probably a bad idea).

yeah i thought of doing that, but we're not supposed to alter 'start-up code', i've made the changes anyway, i heard that global variables are a bad idea generally.

My compiler says data_set undeclared in this (set_insert) function. it so with my above code as well, before altering the parameters.
Any idea why its undeclared?

Originally Posted by AstralZecha

thanks, will do.

i completely overlooked that i have a function for inserting new items set_insert() that i must use:

Code:

int
set_insert(set_t *set, int item, int *set_size)
{
  
  /*if data from file exceeds size*/
  if(data_set->items > 499)
  {
      /*realloc more space*/
  }
  
 /*set_size would refer to the malloc'd size i think, so how can i determine where i am in the array each time i call this function?*/
  data_set->data[?] = item;

}

revised main:

Code:

        while(fscanf(fp, "%d", &input) != EOF)
	{
	    
	    set_insert(data_set, input, 500);
 
	}

data_set undeclared in set_insert, but arent i using it as a parameter?

No, you're using "set" as a parameter.

Originally Posted by AstralZecha

Any idea why its undeclared?

You have to refer to it with the parameter name, not the name of the variable you submitted as the parameter:

Code:

int
set_insert(set_t *set, int item, int *set_size)
{
  if(set->items > 499)

I just noticed these parts of the struct:

Code:

typedef struct 
{
  int *data;
  int items;
  int n_max;
  int lock;
} set_t;

So you do not have to submit i and modify the set_insert() prototype at all. What you should do is in set_create():

Code:

data_set->items = 0;
data_set->n_max = size;

Then in set_insert:

Code:

int set_insert(set_t *set, int item, int *set_size)
{
   if(set->items >= set->n_max)
  {
      /*realloc more space*/
      //  and set n_max to the new size!
  }
  
  set->data[set->items++] = item;
}

Make sense?

of course, thats so simple.

yeah that makes alot of sense, i was wondering what n_max was for.
thanks heaps.

so what would the third parameter int *set_size be for in set_insert() if n_max is the max size of the set and set->items is the current size of the set?

Also, the integers im reading in from the file/s are quite large, going up to something like 5 or 6 million. what would the int's declaration and the fscanf/printing symbols be for numbers that large?

And i am still getting a segfault at the fscanf... is this because of poor malloc'ing?