segmentation fault.

Discussion in 'C Programming' started by Vandana, Mar 18, 2010.

  1. Vandana

    Vandana Guest

    Hello All,

    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!
     
    Vandana, Mar 18, 2010
    #1
    1. Advertising

  2. Vandana <> writes:

    > 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.
     
    Ben Bacarisse, Mar 18, 2010
    #2
    1. Advertising

  3. Vandana

    Hamiral Guest

    pete wrote:
    > #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 ;))
     
    Hamiral, Mar 19, 2010
    #3
  4. Vandana

    bartc Guest

    "Hamiral" <> wrote in message
    news:4ba2c69c$0$21807$...
    > 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
     
    bartc, Mar 19, 2010
    #4
  5. Vandana

    Vandana Guest

    Thanks to all.
     
    Vandana, Mar 19, 2010
    #5
  6. Hamiral <> writes:
    <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.
     
    Ben Bacarisse, Mar 19, 2010
    #6
  7. Vandana

    Seebs Guest

    On 2010-03-18, Vandana <> wrote:
    > 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 /
    http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
    http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
     
    Seebs, Mar 19, 2010
    #7
  8. Vandana

    Hamiral Guest

    Ben Bacarisse wrote:
    > 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, Mar 19, 2010
    #8
  9. Vandana

    Hamiral Guest

    bartc wrote:
    > 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
     
    Hamiral, Mar 19, 2010
    #9
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Alex Hunsley
    Replies:
    17
    Views:
    893
  2. Pud
    Replies:
    0
    Views:
    605
  3. Replies:
    0
    Views:
    569
  4. Ivan Vecerina
    Replies:
    0
    Views:
    506
    Ivan Vecerina
    Jun 29, 2003
  5. Vasileios Zografos

    Re: segmentation fault exception handling

    Vasileios Zografos, Jun 30, 2003, in forum: C++
    Replies:
    5
    Views:
    15,743
    Pete Becker
    Jul 1, 2003
Loading...

Share This Page