segmentation fault.
Vandana
Can anyone please explain why I ma getting segmentation fault in the
following program?
struct employee {
char *name;
struct {
int apmt;
int zip;
} addr;
};
struct employee * populate_employee(char *name, int apt, int zip) {
struct employee *emp;
emp->name = name;
emp->addr.apmt = apt;
emp->addr.zip = zip;
return emp;
}
int main() {
struct employee *emp1;
printf("From the main\n");
emp1 = populate_employee("tom", 5, 90);
printf("address of emp1 is %u\n", emp1);
printf("Values from emp1 %s, %d %d\n", emp1->name, emp1->addr.apmt,
emp1->addr.zip);
return 0;
}
If I remove the printf "From the main" then the program seg faults,
otherwise, the program executes but ends with seg fault.
Thanks for your help!
Ben Bacarisse
> Can anyone please explain why I ma getting segmentation fault in the
> following program?
>
>
> struct employee {
> char *name;
> struct {
> int apmt;
> int zip;
> } addr;
> };
>
> struct employee * populate_employee(char *name, int apt, int zip) {
> struct employee *emp;
> emp->name = name;
> emp->addr.apmt = apt;
> emp->addr.zip = zip;
emp is a pointer but where does it point? Leaving emp uninitialised
means that you can't use emp->anything. To fix this, you need to
decide how you want to manage these objects. The function could be
passed a pointer to a struct to fill in, or you could choose to
dynamically allocate a struct employee using malloc.
> return emp;
> }
<snip>
--
Ben.
pete
#include <stdio.h>
struct employee {
char *name;
struct {
int apmt;
int zip;
} addr;
};
void populate_employee
(struct employee *emp, char *name, int apt, int zip)
{
emp->name = name;
emp->addr.apmt = apt;
emp->addr.zip = zip;
}
int main(void)
{
struct employee empA;
struct employee *emp1 = &empA;
printf("From the main\n");
populate_employee(emp1, "tom", 5, 90);
printf("address of emp1 is %p\n", (void *)emp1);
printf("Values from emp1 %s, %d %d\n",
emp1->name, emp1->addr.apmt, emp1->addr.zip);
return 0;
}
--
pete
Hamiral
> #include <stdio.h>
>
> struct employee {
> char *name;
> struct {
> int apmt;
> int zip;
> } addr;
> };
>
> void populate_employee
> (struct employee *emp, char *name, int apt, int zip)
> {
> emp->name = name;
> emp->addr.apmt = apt;
> emp->addr.zip = zip;
> }
I think this is still dangerous.
What happens if the value passed to parameter name isn't a literal
string, but a dynamically allocated pointer to char, and gets freed
after the call to populate_employee() ?
I would do something like this :
#define NAME_MAX_LEN 50
struct employee {
char name[NAME_MAX_LEN];
struct {
int apmt;
int zip;
} addr;
};
void populate_employee
(struct employee *emp, char *name, int apt, int zip)
{
strncpy(emp->name, name, strlen(name), NAME_MAX_LEN);
emp->name[NAME_MAX_LEN - 1] = '\0';
emp->addr.apmt = apt;
emp->addr.zip = zip;
}
Assuming name is a properly null terminated string, and adding proper
#include's.
Ham (hoping he didn't make any mistake ;))
bartc
"Hamiral" <ham...@hamham.com> wrote in message
news:4ba2c69c$0$21807$426a...@news.free.fr...
> pete wrote:
>> void populate_employee
>> (struct employee *emp, char *name, int apt, int zip)
>> {
>> emp->name = name;
>> emp->addr.apmt = apt;
>> emp->addr.zip = zip;
>> }
>
> I think this is still dangerous.
> What happens if the value passed to parameter name isn't a literal string,
> but a dynamically allocated pointer to char, and gets freed after the call
> to populate_employee() ?
>
> I would do something like this :
>
> #define NAME_MAX_LEN 50
The 50 might be a problem. Most people's names will be less than 50, so
you're wasting space. Then every so often there will be someone with a name
longer than 50, and it will be truncated.
Maybe better to allocate the space locally, of the exact length needed, and
store a copy of the name.
Or insist name is suitable for storing in the record. So the caller
allocates the space, or points to where the name is currently stored.
--
Bartc
Vandana
Thanks to all.
Ben Bacarisse
<snip>
> I would do something like this :
>
> #define NAME_MAX_LEN 50
> struct employee {
> char name[NAME_MAX_LEN];
> struct {
> int apmt;
> int zip;
> } addr;
> };
>
> void populate_employee
> (struct employee *emp, char *name, int apt, int zip)
> {
> strncpy(emp->name, name, strlen(name), NAME_MAX_LEN);
Some thing is wrong here. strncpy takes three arguments. I think you
intended to NAME_MAX_LEN as the third and last argument, though I
slightly prefer:
strncpy(emp->name, name, sizeof emp->name);
> emp->name[NAME_MAX_LEN - 1] = '\0';
> emp->addr.apmt = apt;
> emp->addr.zip = zip;
> }
<snip>
--
Ben.
Seebs
> Can anyone please explain why I ma getting segmentation fault in the
> following program?
Yes.
> struct employee * populate_employee(char *name, int apt, int zip) {
> struct employee *emp;
> emp->name = name;
Where is "emp" pointing when you execute this line? Did you, for
instance, point it *at* anything?
Why, no, you didn't. It's garbage. It may or may not point anywhere
valid, and what it points at may or may not be important.
You might want to look into malloc().
-s
--
Copyright 2010, all wrongs reversed. Peter Seebach / usenet...@seebs.net
http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
Hamiral
> Some thing is wrong here. strncpy takes three arguments. I think you
> intended to NAME_MAX_LEN as the third and last argument, though I
> slightly prefer:
>
> strncpy(emp->name, name, sizeof emp->name);
You're right, I completely messed up the call to strncpy ;) and your
version is more elegant.
Ham
Hamiral
> Maybe better to allocate the space locally, of the exact length needed,
> and store a copy of the name.
I considered this option, but I opted for simplicity and security,
though I messed up the call to strncpy. It would have failed on
compilation ;)
Ham