seg fault in following c code

cdw9997 · 03-09-2009, 03:23 AM

I am getting a seg fault in the following code, do you guys have any ideas how to fix it? of course this isn't the entire code, but if you need the rest I can post it.

Code:

void purchase(FILE **fp, struct Animal *zoo[], int i)
 {
  char action[BUF_SIZE];
  struct Animal *Z[i];
  Z[i] = malloc(1 * sizeof(struct Animal));
  fgets(action, BUF_SIZE, *fp);
  
  sscanf(action, "%s%s%d%d%d%s", Z[i]->species, Z[i]->name, &Z[i]->age, &Z[i]->weight, &Z[i]->id, Z[i]->color);
  
  if(i < 5)
   {
    print(&Z[i], i);
	free( (void *) Z[i]);
	printf("i = %d\n", i);
    purchase(fp, &zoo[BUF_SIZE], i + 1);
   }
  else
   return;

 }

millgates · 03-09-2009, 04:00 AM

and what exactly is this code supposed to do?
1)I don't understand this declaration:

struct Animal *Z[i];

and then allocation:

Z[i] = malloc(1 * sizeof(struct Animal));

you declare i pointers to an Animal while obviously you use just one in the function. Or am i just missing something?
i am also not sure if you can use an argument of the function as an array size in declaration, but if it compiles...

2)And why the recursion? if you need to purchase() 5 animals wouldn't a simple for or while loop be more efficient?

3)is the file you're reading from open?

4) what is the strust Animal *zoo[] argument for?

5)why do you cast (void*) while calling free() ?

that's just for the start...
maybe post other parts of the code: how you call this function and how the arguments you are calling the function with are declared and initialized. and a few comments on what you're trying to do wouldn't hurt

cdw9997 · 03-09-2009, 04:14 AM

Quote:

Originally Posted by millgates

and what exactly is this code supposed to do?
1)I don't understand this declaration:

struct Animal *Z[i];

and then allocation:

Z[i] = malloc(1 * sizeof(struct Animal));

you declare i pointers to an Animal while obviously you use just one in the function. Or am i just missing something?

2)And why the recursion? if you need to purchase() 5 animals wouldn't a simple for or while loop be more efficient?

3)is the file you're reading from open?

4) what is the strust Animal *zoo[] argument for?

5)why do you cast (void*) while calling free() ?

that's just for the start...
maybe post other parts of the code: how you call this function and how the arguments you are calling the function with are declared and initialized. and a few comments on what you're trying to do wouldn't hurt

Recursion is a requirement for this program. I removed the free() statement, and I am not getting a seg fault, but my code is only printing one of the animals correctly, the rest is garbage. Here is the entire code:

Code:

main(int argc, char *argv[])
 {
  struct Animal *zoo[ARRAY_SIZE] = {NULL};
  char action[BUF_SIZE];
  int i;
  if(argc != TWO)
   {
    printf("Command Line Arguments Insufficient");
    exit(1);
   }
   FILE *fp = NULL;
   fp = fopen(argv[ONE], "r");
   if(fp == NULL)
    {
     printf("Cannot Open File");
     exit(1);
    }
   
   
   while(!feof(fp))
    {
     fgets(action, BUF_SIZE, fp);
      switch(action[ONE])
       {
	    case 'p': purchase(&fp, &zoo[BUF_SIZE], i);
         break;
        case 'f': feed(&fp, action);
         break;
        case 't': transfer(&fp, action);
         break;
        default: exit(1);
       }  
      }
    
}
void purchase(FILE **fp, struct Animal *zoo[], int i)
 {
  char A[BUF_SIZE];
  struct Animal *Z[i];
  Z[i] = malloc(1 * sizeof(struct Animal)); 
  fgets(A, BUF_SIZE, *fp);
  
  sscanf(A, "%s%s%d%d%d%s", Z[i]->species, Z[i]->name, &Z[i]->age, &Z[i]->weight, &Z[i]->id, Z[i]->color);
  
  if(i < 5)
   {
    print(&Z[i], i);
	
	printf("i = %d\n", i);
	
    purchase(fp, &zoo[BUF_SIZE], i + 1);
   }
  else
   return;

 }
void feed(FILE **fp, char action[])
 {
  printf("feed function\n");
 }
void transfer(FILE **fp, char action[])
 {
  printf("transfer function\n");
 }
void print(struct Animal *Z[], int i)
 {
  char string1[STRING_SIZE] = "Species";
  char string2[STRING_SIZE] = "Name";
  char string3[STRING_SIZE] = "Age";
  char string4[STRING_SIZE] = "Weight";
  char string5[STRING_SIZE] = "id";
  char string6[STRING_SIZE] = "Color";

  printf("%5s%7s%9s%11s%13s%15s\n", string1, string2, string3, string4, string5, string6);
  printf("%5s%7s%9d%11d%13d%15s\n", Z[i]->species, Z[i]->name, Z[i]->age, Z[i]->weight, Z[i]->id, Z[i]->color);
 }

millgates · 03-09-2009, 04:53 AM

Quote:

Originally Posted by cdw9997

Recursion is a requirement for this program.

aah, homework

) for a while i thought you actually work in a zoo

)

first, i would stop declaring *Z[i] inside of purchase()
from what i see you don't need an array there, a single pointer would be enough

then i would stop feeding purchase() with *zoo[] since you don't use it either

and last, in print() i would also not use array of pointers to print the one pointer specified by i, passing only the pointer to animal you want to print will suffice

when you do this i bet the code will be much easier to debug

millgates · 03-09-2009, 05:22 AM

I have reconstructed missing parts of code (such as #defines and animal definitions) and created fictional file
then i have done as i said in my previous post and it runs just fine(at least the purchase function, i haven't tested the rest of it)

just a few things:
1) there's no point in i variable in main() - it's zero by default and it never changes - you may as well pass 0 into purchase()
2) Z in purchase doesen't have to be a pointer at all, just declare it as struct Animal Z
and you'll save yourself the malloc call
3) if you have problems with purchase()ing, check that your text file is in the right format:
action
6 animals
action
6 animals
etc.
otherwise it probably won't do what you want it to do

cdw9997 · 03-09-2009, 11:53 AM

Here is my input file

Code:

*purchase
Bear Jane  4 150   4002 black
Monkey  George  2  12 3009  black
Giraffe  Tiny  3  340  2001  yellow
Condor  Alice 3 12 5001 red
Coyote  John Jesse 2  3 6002  grey
Fox Road Ranger 2 1 8001 red
*feed
Jane 12.2 2.1
George 4.0  3.2
Tiny 1.3 0.5
Alice 0.3 0.2
John Jesse 1.2 0.4
Road Ranger 0.8 0.2
*transfer
John Jesse
Alice
Jack
*purchase
Bird Road Runner 1 1 7001 yellow
Fox Night Rover 1 2 8002 silver
Giraffe Sally  2  300   2002  brown

I have to read in these values in to an array of 5 structure pointers. I have to purchase, feed, transfer, and print the entire zoo recursively.

Here is my constants file:

Code:

#include<stdio.h>
#include<stdlib.h>
#include<ctype.h>
#include<string.h>
#define BUF_SIZE 81
#define ARRAY_SIZE 5
#define TWO 2
#define ONE 1
#define IZERO 0
#define STRING_SIZE 10


struct Animal
 {
  char species[BUF_SIZE];
  char name[BUF_SIZE];
  int age;
  int weight;
  int id;
  char color[BUF_SIZE];
  double food;
  double water;
 };

void purchase(FILE **fp, struct Animal *[], int i);
void feed(FILE **fp, char action[]);
void transfer(FILE **fp, char action[]);
void print(struct Animal *[], int i);

and here is my driver file:

Code:

#include "hw2constants.c"

main(int argc, char *argv[])
 {
  struct Animal *zoo[ARRAY_SIZE] = {NULL};
  char action[BUF_SIZE];
  int i;
  if(argc != TWO)
   {
    printf("Command Line Arguments Insufficient");
    exit(1);
   }
   FILE *fp = NULL;
   fp = fopen(argv[ONE], "r");
   if(fp == NULL)
    {
     printf("Cannot Open File");
     exit(1);
    }
   
   
   while(!feof(fp))
    {
     fgets(action, BUF_SIZE, fp);
      switch(action[ONE])
       {
	    case 'p': purchase(&fp, zoo, i);
         break;
        case 'f': feed(&fp, action);
         break;
        case 't': transfer(&fp, action);
         break;
        default: exit(1);
       }  
      }
    
}
void purchase(FILE **fp, struct Animal *zoo[], int i)
 {
  char A[BUF_SIZE];
 
  zoo[i] = malloc(BUF_SIZE * sizeof(struct Animal)); 
  fgets(A, BUF_SIZE, *fp);
  
  sscanf(A, "%s%s%d%d%d%s", zoo[i]->species, zoo[i]->name, &zoo[i]->age, &zoo[i]->weight, &zoo[i]->id, zoo[i]->color);
  
  if(i < 5)
   {
    print(&zoo[i], i);
	free( (void *) zoo[i]);
	printf("i = %d\n", i);
	
    purchase(fp, zoo, i + 1);
   }
  else
   return;

 }
void feed(FILE **fp, char action[])
 {
  printf("feed function\n");
 }
void transfer(FILE **fp, char action[])
 {
  printf("transfer function\n");
 }
void print(struct Animal *zoo[], int i)
 {
  char string1[STRING_SIZE] = "Species";
  char string2[STRING_SIZE] = "Name";
  char string3[STRING_SIZE] = "Age";
  char string4[STRING_SIZE] = "Weight";
  char string5[STRING_SIZE] = "id";
  char string6[STRING_SIZE] = "Color";

  printf("%5s%7s%9s%11s%13s%15s\n", string1, string2, string3, string4, string5, string6);
  printf("%5s%7s%9d%11d%13d%15s\n", zoo[i]->species, zoo[i]->name, zoo[i]->age, zoo[i]->weight, zoo[i]->id, zoo[i]->color);
 }

johnsfine · 03-09-2009, 01:28 PM

Quote:

Originally Posted by millgates

and what exactly is this code supposed to do?

Two replies by the OP later and that key question is not answered.

There is a serious error in that aspect of the original code, that the OP should either fix or explain enough of the intent that someone can give useful advice on what to fix.

Quote:

1)I don't understand this declaration:

struct Animal *Z[i];

and then allocation:

Z[i] = malloc(1 * sizeof(struct Animal));

you declare i pointers to an Animal while obviously you use just one in the function. Or am i just missing something?

It is a more serious error than you said (so maybe you missed something).

The OP allocated i pointers and only used one. But the one that was used was just outside the range of the i that were allocated, so using it corrupts some other item on the stack.

cdw9997 · 03-09-2009, 10:38 PM

Here are the specific instructions for this assignment:

define in the constants file a structure representing an animal
open an input file
read data from the input file
print the contents of the principal data structures

Four separate functions are then to be used to

D. acquire animals for the zoo, recursively, creating an object of the structure to represent
one animal as the data is read in from the input file for that one animal
E. display the data for the animals created so far, recursively (this report should use a
minimum # of lines)
F. feed and water the animals (recursively)
G. transfer animals out of the zoo, recursively

johnsfine · 03-10-2009, 08:13 AM

Quote:

Originally Posted by millgates

and what exactly is this code supposed to do?

After quoting that, I worried a little that the quote might be misinterpreted out of context.

And it was:

Quote:

Originally Posted by cdw9997

Here are the specific instructions for this assignment:

"what exactly is this code supposed to do?" Meant the specific code in the first post that misuses that array of structure pointers. It did not mean "what is the whole homework assignment?"

As long as you quoted the whole assignment, we do get to see the very arbitrary requirement for "recursively" in tasks that don't seem to have any natural reason to be recursive.

I hope you got some guidance in class about what your instructor is really looking for, because quoted out of the context of that class, it looks like a very badly specified assignment.

Even if someone here thinks they understand the assignment instructions, it is very much out of policy for anyone here to do your homework for you.

Posting your code for your attempt at part of the task, with questions about specific problems, as you did in the first post, was definitely the right way to get help here.

If your attempt had been close, I'm sure you would have the right help by now.

At this point, I think you must clarify, at least to yourself, how you interpret the assignment instructions and what you expect each function of your program to do.

cdw9997 · 03-10-2009, 10:06 AM

Quote:

Originally Posted by johnsfine

After quoting that, I worried a little that the quote might be misinterpreted out of context.

And it was:

"what exactly is this code supposed to do?" Meant the specific code in the first post that misuses that array of structure pointers. It did not mean "what is the whole homework assignment?"

As long as you quoted the whole assignment, we do get to see the very arbitrary requirement for "recursively" in tasks that don't seem to have any natural reason to be recursive.

I hope you got some guidance in class about what your instructor is really looking for, because quoted out of the context of that class, it looks like a very badly specified assignment.

Even if someone here thinks they understand the assignment instructions, it is very much out of policy for anyone here to do your homework for you.

Posting your code for your attempt at part of the task, with questions about specific problems, as you did in the first post, was definitely the right way to get help here.

If your attempt had been close, I'm sure you would have the right help by now.

At this point, I think you must clarify, at least to yourself, how you interpret the assignment instructions and what you expect each function of your program to do.

I just wanted to see if anyone could identify the line of code that was causing the seg fault and how to fix it, I was not asking anyone to do the assignment for me.

millgates · 03-10-2009, 12:05 PM

the problem that you are accessing memory that doesn't belong to you(or at least the specified variable)
i guess that reading from it usually causes garbage data while writing to it causes segfaults

my advice to you is:
1) tidy up your code a bit
2) stop for a while and think: decide(as already said by johnsfine) what exactly do you want every function in your code to do, what to return, and what arguments will you need to supply it with, so you don't create redundant parameters, allocate memory that you don't use.
if you can't figure it out in your mind, draw it on a piece of paper first
3) make sure your comfortable with the basic concept of pointers, arrays, arrays of pointers, etc. and how to use them
4) make sure that you allocate memory to your pointers, that you write to that memory and read from that memory(and not somewhere else)
5) test your code once in a while, so you know about an error when you make it and not when the code is complete
6) comment comment and once more comment your code
7) i have noticed in your text file you use names containing spaces. Guess what sscanf will read from such line?
8) be patient. There's no coding without debugging. You'll get to it. And by correcting your mistakes you can often learn much more than by not doing them at all.

GazL · 03-10-2009, 07:34 PM

Quote:

Originally Posted by millgates

i am also not sure if you can use an argument of the function as an array size in declaration, but if it compiles...

I'm currently teaching myself C and, I didn't expect you could do that either, so I wrote myself a little test...

Code:

void func1(int i)
{

int j;
int array[i];

for ( j=0 ; j < i ; j++ )
    array[j] = 100;

printf("array[3] is %d\n",array[3]);

return;
}

It appears that this works as long as you don't declare the array as static, which I've found will give you a;

Quote:

error: storage size of 'array' isn't constant

I was expecting to have to malloc() the space for the array in this situation but presumably, when the array is defined as automatic, the space is allocated on the stack at runtime? If so, I assume that stack size might be an issue with a large array and so this is generally not a good idea.

If someone could confirm or correct my assumptions here it'd be a big help to my understanding and I'd appreciate it.

edit: just upped the value of i passed to 500000 and that does generates a segmentation fault.

johnsfine · 03-10-2009, 07:49 PM

Quote:

Originally Posted by GazL

I'm currently teaching myself C and, I didn't expect you could do that either, so I wrote myself a little test...

Other posts have said that is a feature of C99 that some C and/or C++ compilers let you use. I haven't bothered to try to find which standards and/or compiler permit it.

Quote:

when the array is defined as automatic, the space is allocated on the stack at runtime? If so, I assume that stack size might be an issue with a large array

Correct.

Quote:

edit: just upped the value of i passed to 500000 and that does generates a segmentation fault.

A 2MB stack limit?? Seems small. What environment did you build in?

GazL · 03-10-2009, 08:02 PM

Quote:

Originally Posted by johnsfine

Other posts have said that is a feature of C99 that some C and/or C++ compilers let you use. I haven't bothered to try to find which standards and/or compiler permit it.

Correct.

A 2MB stack limit?? Seems small. What environment did you build in?

Thanks John, sorry, that was a typo, I think it must have been 5000000.
I've just ran through it and the first value that triggers a segfault is actually, 2094871.