Allocate memory to char * variables in structure

[dtschoepe]

Hi,

I have a homework project I am working on, so be forwarned, I'm new to
C programming. But anyway, having some trouble with a memory
allocation issue related to a char * that is a variable inside of a
structure. I keep getting segmentation fault errors and I am having
trouble understanding why. Here's the parts of the code in question...

This is part of the .h file where the struct us defined...

enum color { COLORS };
typedef enum color Color;

struct person
{
char *firstName;
char *lastName;
char *hobby;
Color favColor;
char sex;
int age;
};
typedef struct person Person;

struct node
{
struct node *next;
Person client;
};
typedef struct node Node;

Now here is the function that I am having trouble with... If you look
down to the section where this line is found...

member->firstName = (char *)calloc(strlen (fname)+1, sizeof(char));

The compiler stops there and generates the seg fault message. Here's
the full function. By the way I have tested the first part of the
function, the file pointer exists and the output to stdout from printf
is working ok until I get the line above. Then the program crashes.

Am I trying to allocate the memory in the wrong way?
Person *createMember(F ILE *infile)
{
char fname[30];
char lname[30];
char sex[10];
char age[10];
char color[20];
char hobby[30];
char *colorS;
Person *member;

if (!feof(infile))
{
fgets(fname, 30, infile);
fgets(lname, 30, infile);
fgets(sex, 10, infile);
fgets(age, 10, infile);
fgets(color, 20, infile);
fgets(hobby, 30, infile);
}

else
return NULL;
printf("top of createMember\n" );
/* read member attributes from file */
fgets(fname, sizeof fname, infile);
fname[strlen(fname)-1] = '\0';
fgets(lname, sizeof lname, infile);
lname[strlen(lname)-1] = '\0';
fgets(sex, sizeof sex, infile);
sex[strlen(sex)-1] = '\0';
fgets(age, sizeof age, infile);
age[strlen(age)-1] ='\0';
fgets(color, sizeof color, infile);
color[strlen(color)-1] = '\0';
fgets(hobby, sizeof hobby, infile);
hobby[strlen(hobby)-1] = '\0';
printf("got through fgets statements.\n") ;

/* create memory allocation for member attributes */
member->firstName = (char *)calloc(strlen (fname)+1, sizeof(char));
printf("calloc firstName\n");
member->lastName = (char *)malloc(sizeof (fname) * sizeof(char));
member->hobby = (char *)malloc(sizeof (hobby) * sizeof(char));
colorS = (char *)malloc(sizeof (color) * sizeof(char));
printf("got past malloc statements\n");

/* store member attributes */
strcpy(member->firstName, fname);
printf("fname processed\n");
sscanf(lname, "%s", member->lastName);
sscanf(sex, "%c", member->sex);
sscanf(age, "%d", member->age);
strncpy(colorS, color, sizeof(color));
member->favColor = readColor(color S);
sscanf(hobby, "%s", member->hobby);
printf("bottom of createMember\n" );

return member;
}

David

[Ian Collins]

dt*******@gmail .com wrote:

Hi,

I have a homework project I am working on, so be forwarned, I'm new to
C programming. But anyway, having some trouble with a memory
allocation issue related to a char * that is a variable inside of a
structure. I keep getting segmentation fault errors and I am having
trouble understanding why. Here's the parts of the code in question...

>

Now here is the function that I am having trouble with... If you look
down to the section where this line is found...

member->firstName = (char *)calloc(strlen (fname)+1, sizeof(char));

Never cast the return of calloc/malloc/realloc. Also sizeof(char) is by
definition 1. Remove the casts and make sure you have included <stdlib.h>

The compiler stops there and generates the seg fault message.

I assume you should have said the program crashes when you run it?

sscanf(sex, "%c", member->sex);
sscanf(age, "%d", member->age);

the above should use &member->sex and &ember->age.

--
Ian Collins.

[santosh]

dt*******@gmail .com wrote:

Hi,

I have a homework project I am working on, so be forwarned, I'm new to
C programming. But anyway, having some trouble with a memory
allocation issue related to a char * that is a variable inside of a
structure. I keep getting segmentation fault errors and I am having
trouble understanding why. Here's the parts of the code in question...

This is part of the .h file where the struct us defined...

enum color { COLORS };

Only one enumeration constant?

typedef enum color Color;

struct person
{
char *firstName;
char *lastName;
char *hobby;
Color favColor;
char sex;
int age;
};
typedef struct person Person;

struct node
{
struct node *next;
Person client;
};
typedef struct node Node;

Now here is the function that I am having trouble with... If you look
down to the section where this line is found...

member->firstName = (char *)calloc(strlen (fname)+1, sizeof(char));

sizeof(char) is always one in C. Also no need to cast the return value
of calloc, (or malloc or realloc), as it can hide certain mistakes.

The compiler stops there and generates the seg fault message.

You mean compilation stops with a segmentation fault. If so the
compiler is broken.

Here's
the full function. By the way I have tested the first part of the
function, the file pointer exists and the output to stdout from printf
is working ok until I get the line above. Then the program crashes.

Am I trying to allocate the memory in the wrong way?
Person *createMember(F ILE *infile)
{
char fname[30];
char lname[30];
char sex[10];
char age[10];
char color[20];
char hobby[30];
char *colorS;
Person *member;

if (!feof(infile))

In C, feof and ferror are used _after_ a call to one of the I/O
functions have reported failure.

{
fgets(fname, 30, infile);
fgets(lname, 30, infile);
fgets(sex, 10, infile);
fgets(age, 10, infile);
fgets(color, 20, infile);
fgets(hobby, 30, infile);

Check all these calls for failure. Are you also sure that the various
fields will be less than the array sizes?

}

else
return NULL;
printf("top of createMember\n" );
/* read member attributes from file */
fgets(fname, sizeof fname, infile);

Why are you overwriting fname again?

fname[strlen(fname)-1] = '\0';

This will already be done for you by fgets.

fgets(lname, sizeof lname, infile);
lname[strlen(lname)-1] = '\0';
fgets(sex, sizeof sex, infile);
sex[strlen(sex)-1] = '\0';
fgets(age, sizeof age, infile);
age[strlen(age)-1] ='\0';
fgets(color, sizeof color, infile);
color[strlen(color)-1] = '\0';
fgets(hobby, sizeof hobby, infile);
hobby[strlen(hobby)-1] = '\0';
printf("got through fgets statements.\n") ;

All these statements are duplicates of the what you'd written earlier
within the if construct. The overwrite values read earlier with
possibly wrong values. Is this really what you want?

Also check the fgets statements for failure. Don't simply assume to
have got through safely.

/* create memory allocation for member attributes */
member->firstName = (char *)calloc(strlen (fname)+1, sizeof(char));

Here's your main error. member is pointer to a Person type, (which is
actually a struct of type person), but as yet it doesn't point to
valid instance of a Person object. You need to define a Person object,
set member to point to it and then do the memory allocation for the
fields like:

Person p1;
member = &p1;
/* ... */

printf("calloc firstName\n");
member->lastName = (char *)malloc(sizeof (fname) * sizeof(char));
member->hobby = (char *)malloc(sizeof (hobby) * sizeof(char));
colorS = (char *)malloc(sizeof (color) * sizeof(char));

Same as above. Also don't cast return value of *alloc. I'd write these
statements as:

printf("got past malloc statements\n");

Without error checking.

/* store member attributes */
strcpy(member->firstName, fname);
printf("fname processed\n");
sscanf(lname, "%s", member->lastName);

Why strcpy for firstName and sscanf for lastName?

sscanf(sex, "%c", member->sex);
sscanf(age, "%d", member->age);
strncpy(colorS, color, sizeof(color));
member->favColor = readColor(color S);
sscanf(hobby, "%s", member->hobby);
printf("bottom of createMember\n" );

return member;
}

Make sure your Person object is not a local one, since, if so, it'll
get destroyed when execution leaves this function. Either dynamically
allocate the object or make it a file scope one.

[dtschoepe]

On Mar 4, 10:02 pm, Ian Collins <[e-mail] mwrote:

dtscho...@gmail .com wrote:

Hi,

I have a homework project I am working on, so be forwarned, I'm new to
C programming. But anyway, having some trouble with a memory
allocation issue related to a char * that is a variable inside of a
structure. I keep getting segmentation fault errors and I am having
trouble understanding why. Here's the parts of the code in question...

Now here is the function that I am having trouble with... If you look
down to the section where this line is found...

member->firstName = (char *)calloc(strlen (fname)+1, sizeof(char));

Never cast the return of calloc/malloc/realloc. Also sizeof(char) is by
definition 1. Remove the casts and make sure you have included <stdlib.h>

The compiler stops there and generates the seg fault message.

I assume you should have said the program crashes when you run it?

sscanf(sex, "%c", member->sex);
sscanf(age, "%d", member->age);

the above should use &member->sex and &ember->age.

OK, am I correct in understanding that I should be changing the code
as follows?

member->firstName = calloc(strlen(f name)+1, sizeof(char));

Notice I removed the (char *) from in front of calloc. Actually,
leaving it there is how any example in class had it. So I am confused.

I did also mean the program crashes, not the compiler stops. I am too
tired to think clearly tonight.

By the way, I just recompiled and ran the program by removing the
(char *) cast, but the same seg fault error is still there. I will
need to review my code more.

Also, I am including the stdlib.h library in my header.

David

[dtschoepe]

On Mar 4, 10:04 pm, "santosh" <santosh....@gm ail.comwrote:

Why are you overwriting fname again?

fname[strlen(fname)-1] = '\0';

This will already be done for you by fgets.

Well, according to my handy dandy textbook, Programming in C by
Stephen Kochan, the fgets function will read everything into the array
including the newline character (\n), so I am overwriting the newline
character with the \0 character so I can eliminate the newline
character.

I am reading all the data from a file and each set of data ends with
new line character.

David

[Ian Collins]

dt*******@gmail .com wrote:

>

OK, am I correct in understanding that I should be changing the code
as follows?

member->firstName = calloc(strlen(f name)+1, sizeof(char));

Or just

member->firstName = calloc(strlen(f name)+1, 1);

And take note of the critical bug santosh spotted that I didn't - you
have not allocated any memory for member.

Notice I removed the (char *) from in front of calloc. Actually,
leaving it there is how any example in class had it. So I am confused.

This is a common mistake. All the cast does is suppress important error
messages if you forget to include the header, nothing else.

--
Ian Collins.

[dtschoepe]

On Mar 4, 10:04 pm, "santosh" <santosh....@gm ail.comwrote:

>

else
return NULL;
printf("top of createMember\n" );
/* read member attributes from file */
fgets(fname, sizeof fname, infile);

Why are you overwriting fname again?

You got me there. I wrote that code at two different times and
obviously made a boo-boo. I have deleted the second set of fgets()
statements. Although in the test this didn't hurt anything, I have
enough data in the file to cover the mistake.

I'm working on your other suggestions but to this point I'm still
getting the segfault when the program runs. I think this has something
to do with your reference to me not creating a proper Person.

Here's your main error. member is pointer to a Person type, (which is
actually a struct of type person), but as yet it doesn't point to
valid instance of a Person object. You need to define a Person object,
set member to point to it and then do the memory allocation for the
fields like:

Person p1;
member = &p1;

If I define member using...

Person member;

What is the difference if I use member or p1? In your example, you
didn't associate member with a type. So I'm not sure what relevance
your example has? That being said, your point is a valid one, and I
would like to learn the correct way this is done. Hopefully I can
figure it out, but I just need to figure how the syntax of where I'm
wrong.

David

[dtschoepe]

Santos and Ian,

Thanks for your help. I did review your comments again and see where
my problems were. I am now making progress with the program.

David

[Ian Collins]

dt*******@gmail .com wrote:

Santos and Ian,

Thanks for your help. I did review your comments again and see where
my problems were. I am now making progress with the program.

Good luck!

--
Ian Collins.

[santosh]

dt*******@gmail .com wrote:

On Mar 4, 10:04 pm, "santosh" <santosh....@gm ail.comwrote:

<snip>

I'm working on your other suggestions but to this point I'm still
getting the segfault when the program runs. I think this has something
to do with your reference to me not creating a proper Person.

>Here's your main error. member is pointer to a Person type, (which is
>actually a struct of type person), but as yet it doesn't point to
>valid instance of a Person object. You need to define a Person object,
>set member to point to it and then do the memory allocation for the
>fields like:
>
Person p1;
member = &p1;

If I define member using...

Person member;

>From your first post to this thread:

>>>>>>>>>>>>

Person *createMember(F ILE *infile)
{
char fname[30];
char lname[30];
char sex[10];
char age[10];
char color[20];
char hobby[30];
char *colorS;
Person *member;

if (!feof(infile))
{
<<<<<<<<<<<<<

As you can see, you've declared member as a _pointer_ to an object of
type Person. Now you're saying member is a Person object, not a
pointer. If so, all your accesses to the fields of member by using the
structure indirection operator, (->), is wrong. It can only be used
with pointers to structures. If member is an actual struct object then
use the dot operator to access fields like:

member.firstNam e = /* whatever */

What is the difference if I use member or p1?

No difference. But there *is* a difference between Person *member and
Person member. The former is a pointer to a Person object, while the
latter *is* a Person object.

In your example, you
didn't associate member with a type. So I'm not sure what relevance
your example has? That being said, your point is a valid one, and I
would like to learn the correct way this is done. Hopefully I can
figure it out, but I just need to figure how the syntax of where I'm
wrong.

This is basic structure stuff. Your textbook should have the
information. If not, then it isn't a good C textbook.