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

[Flash Gordon]

santosh wrote, On 05/03/07 06:04:

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

<snip>

> /* 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;

<snip>

You might confuse the OP my showing this method.

> 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.

Making it file scope is incredibly bad advice, please don't suggest to
newbies using "globals", they are needed sometimes, but definitely not
in most situations where a newbie might use them. Making it a static
(which would also would) would be bad advice as well.

Either it should be dynamically allocated or a pointer to a structure
provided by the calling routine should be passed in, depending on
exactly what the OP is doing.
--
Flash Gordon

[Flash Gordon]

dt*******@gmail .com wrote, On 05/03/07 05:35:

>
I have a homework project I am working on,

Thank you for being honest about it being homework.

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));

Firstly you do not need to cast the result of calloc/malloc/realloc. If
the compiler complains without the cast then that means you have done
something else wrong, either compiling the code as C++ (where the cast
would be needed but you should not really use malloc) or failing to
#include <stdlib,h>

>
The compiler stops there and generates the seg fault message. Here's

I don't think it is the compiler that is stopping with a seg fault,
rather it is your program stopping with a seg fault when you run it.
This is a very important distinction since the compiler seg faulting
would mean either a compiler bug or a hardware fault rather than a bug
in your program.

Precision is important in programming and in reporting problems.

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?

Yes, but not quite wrong in the way you think.

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;

member is a pointer to a struct, but currently it does not point anywhere.

if (!feof(infile))

This is almost certainly incorrect use of feof. In C feof does not tell
you that you are at the end of the file, it tells you if your failure to
read from the file was due to being at the end of file. I.e. it only
helps after the event, not before.

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

You should check whether any of the above fail. One possible "simple"
method would be getting rid of the if above and using:
if (!(fgets(...) &&
fgets(...) && ... )
return NULL;

This relies on:
1) fgets is defined as returning a null pointer on failure doe to either
EOF or some other error condition and a non-null pointer otherwise
2) A null pointer in a boolean context is treated as "false" and a
non-null pointer as "true"
3) It saves needless calls to fgets because as soon as one fails the
"short-circuit" rules of && will means that the rest are not done.

All of the above are guaranteed by the C standard.

}

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

Why are you reading over fname again? Where is your error checking?

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

What if the line was too long? You need to check that as well.

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));

Remember I said that member did not point anywhere yet? Well, it still
does not. You need to allocate space for your struct and assign the
pointer to that space to member first.

You also need to check if malloc/calloc/realloc failed.

Finally, since you are about to write a string in to the allocated space
there is no point using calloc, you might as well use malloc.

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);

The above calls to sscanf are wrong, and in any case you do not need the
flexibility of sscanf. Read up on scanf, and also on the strto*
functions and remember that a string is an array of characters so you
can access an individual character using array access.

strncpy(colorS, color, sizeof(color));

ColorS does not point anywhere yet so this is a problem. In any case,
strncpy is almost certainly not what you want.

member->favColor = readColor(color S);
sscanf(hobby, "%s", member->hobby);
printf("bottom of createMember\n" );

return member;
}

I suggest you read the comp.lang.c FAQ at http://c-faq.com/
Start with sections 6, 7 and 12 which address some of your
misconceptions, then go through the rest as and when you have time. Also
keep it bookmarked as you should go back to it to look for solutions to
your problems before posting here. If you can't find, or can't
understand, the answer then ask for help.
--
Flash Gordon

[santosh]

Flash Gordon wrote:

santosh wrote, On 05/03/07 06:04:

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

<snip>

/* 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;

<snip>

You might confuse the OP my showing this method.

Why?

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.

Making it file scope is incredibly bad advice, please don't suggest to
newbies using "globals", they are needed sometimes, but definitely not
in most situations where a newbie might use them. Making it a static
(which would also would) would be bad advice as well.

Point taken.

[CBFalconer]

"dt*******@gmai l.com" wrote:

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.

But you don't handle the case where those lines are overlong. This
is an ideal place to use ggets (but not gets), which will handle
allocation and consistently suppress the terminal '\n'. You can
find it at:

<http://cbfalconer.home .att.net/download/>

--
<http://www.cs.auckland .ac.nz/~pgut001/pubs/vista_cost.txt>
<http://www.securityfoc us.com/columnists/423>

"A man who is right every time is not likely to do very much."
-- Francis Crick, co-discover of DNA
"There is nothing more amazing than stupidity in action."
-- Thomas Matthews

[Flash Gordon]

santosh wrote, On 05/03/07 08:12:

Flash Gordon wrote:

>santosh wrote, On 05/03/07 06:04:

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

<snip>

>>> /* 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;

<snip>

You might confuse the OP my showing this method.

Why?

<snip>

Because it is not the correct solution to the problem as you correctly
stated further down but at this point in the post (and for a fair bit
their after) the OP would probably be assuming that it was the correct
solution.

My experience with inexperienced people is that if you present an
incorrect solution and then a fair time later get around to saying it is
incorrect you confuse them, since they have been assuming it to be the
correct solution to their problem.
--
Flash Gordon

[Daniel Rudy]

At about the time of 3/4/2007 9:35 PM, dt*******@gmail .com stated the
following:

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?

Yep. Just use malloc(3).

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

Your statements like malloc(sizeof(f name) * sizeof(char)); can be
reduced to malloc(sizeof(f name)); since char is garunteed to be size 1.

--
Daniel Rudy

Email address has been base64 encoded to reduce spam
Decode email address using b64decode or uudecode -m

Why geeks like computers: look chat date touch grep make unzip
strip view finger mount fcsk more fcsk yes spray umount sleep

[dtschoepe]

On Mar 5, 1:19 am, CBFalconer <cbfalco...@yah oo.comwrote:

"dtscho...@gmai l.com" wrote:

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.

But you don't handle the case where those lines are overlong. This
is an ideal place to use ggets (but not gets), which will handle
allocation and consistently suppress the terminal '\n'. You can
find it at:

<http://cbfalconer.home .att.net/download/>

--

In this case the instructor is guaranteeing our input will meet
certain specifications. I agree normally I would need to check for
oversized input.

David

[Flash Gordon]

dt*******@gmail .com wrote, On 06/03/07 05:45:

<snip>

In this case the instructor is guaranteeing our input will meet
certain specifications. I agree normally I would need to check for
oversized input.

My experience is that whenever one is given such a guarantee you end up
receiving input that through some error does not meet the specification.
Therefore it is best to at least do a check a check and some rudimentary
handling of bad data. As this is just homework, something as simple as
aborting the program with a suitable error message if the data is out of
spec would do. You might even get extra points for it!
--
Flash Gordon