jonathanberi
jonathanberi

Reputation: 1947

Segmentation fault: 11 when returning stuct via function pointer in struct

I'm trying to emulate a constructor. I'm attempting to do this by having a parent Struct have a function pointer that returns a pointer to a child struct. Any help would be much appreciated.

#include <stdio.h>
#include "stdlib.h"

typedef struct child_t {

  char *name;

} Child;

typedef struct parent_t {

  Child (*NewChild)();

} Parent;

Child *NewChild() {
  Child *child = malloc(sizeof(Child));

  child->name = "foo";

  return child;
}

int 
main()
{
  Parent parent;

  Child child = parent.NewChild();
}

Upvotes: 5

Views: 392

Answers (3)

anderas
anderas

Reputation: 5844

It seems like you haven't initialized parent.NewChild. Try setting that to the function pointer before calling it, like this: parent.NewChild = NewChild;.

Also, aren't you missing the * in Child *child = parent.NewChild();?

Addendum: By the comments, there is also an error/conflict in the declaration of Parent: Its NewChild member function is declared to return a Child, while the free method NewChild returns a Child*. So declare the member function pointer as Child * (*NewChild)();.

Upvotes: 9

R Sahu
R Sahu

Reputation: 206567

I see the following problems in your code.

Mismatching function and function pointer

In Parent, you have defined NewChild as:

Child (*NewChild)();

That says NewChild is a pointer to a function that takes no arguments and returns a Child object.

The function that you have defined:

Child *NewChild() { ...

Takes no arguments and returns a pointer to a Child, not an object of Child.

You'll have to decide which one needs to be fixed. To fix the function, make its return type Child.

Child NewChild() { ...

Uninitialized Child of parent

You have left the value of parent.Child uninitialized yet you are trying to use it.

Setting the member of Child

You use the following code to set the name of child.

child->name = "foo";

This may or may not be a problem depending on how you use child->name since child->name points to some global data in the computers read-only memory. It will be better to make a copy of "foo".

Here's my suggestion for updating your code:

Child NewChild() 
{
   Child child;

   // strdup is available on some platforms but it can 
   // be implemented easily.
   child->name = strdup("foo");

   return child;
}

int main()
{
   Parent parent;
   parent.NewChild = NewChild;

   Child child = parent.NewChild();
}

Upvotes: 1

Vlad from Moscow
Vlad from Moscow

Reputation: 310930

I think you mean the following

#include <stdio.h>
#include <stdlib.h>

typedef struct child_t {

  char *name;

} Child;

typedef struct parent_t {

    Child * ( *NewChild )( void );

} Parent;

Child * NewChild( void ) 
{
    Child *child = malloc( sizeof( Child ) );

    child->name = "foo";

    return child;
}

int main( void )
{
    Parent parent = { NewChild };

    Child *child = parent.NewChild();

    puts( child->name );

    free( child );
}    

The program output is

foo

That is you should declare the function correctly. Its return type must be a pointer. So you must to declare the function pointer correctly in the structure definition

typedef struct parent_t {

  Child (*NewChild)();
  ^^^^^^
} Parent;

And you need to initialize the object of type Parent.

Parent parent = { NewChild };

otherwise it has an indeterminate value.

Upvotes: 3

Related Questions