Segmentation fault in cast struct in c

Question

In an attempt to encapsulate struct members (in a similar way as discussed in this question), I created the code below.

In the code below, I have a c-struct, which contains methods to access members of the struct which are hidden (by being cast into a struct otherwise the same but without the hidden properties)

#include <stdio.h>

typedef struct class {
    int publicValue;
    int (*getPV)();
    void (*setPV)(int newPV);
} class;

typedef struct classSource {
    int publicValue;
    int apv;
    int (*getPV)();
    void (*setPV)(int newPV);
    int PV;
} classSource;

class class_init() {
    classSource cs;
    cs.publicValue = 15;
    cs.PV = 8;
    int class_getPV() {
        return cs.PV;
    };
    void class_setPV(int x) {
        cs.PV = x;
    };
    cs.getPV = class_getPV;
    cs.setPV = class_setPV;
    class *c = (class*)(&cs);
    return *c;
}


int main(int argc, const char * argv[]) {
    class c = class_init();
    c.setPV(3452);
    printf("%d", c.publicValue);
    printf("%d", c.getPV());
    return 0;
}

When I run this, I get a segmentation fault error. However, I noticed that if I comment out certain lines of code, it (seems) to work okay:

#include <stdio.h>

typedef struct class {
    int publicValue;
    int (*getPV)();
    void (*setPV)(int newPV);
} class;

typedef struct classSource {
    int publicValue;
    int apv;
    int (*getPV)();
    void (*setPV)(int newPV);
    int PV;
} classSource;

class class_init() {
    classSource cs;
    cs.publicValue = 15;
    cs.PV = 8;
    int class_getPV() {
        return cs.PV;
    };
    void class_setPV(int x) {
        cs.PV = x;
    };
    cs.getPV = class_getPV;
    cs.setPV = class_setPV;
    class *c = (class*)(&cs);
    return *c;
}


int main(int argc, const char * argv[]) {
    class c = class_init();
    c.setPV(3452);
    //printf("%d", c.publicValue);
    printf("%d", c.getPV());
    return 0;
}

I presume that it might have something to do with using the initializer to add the getter and setter methods to the struct, as those might overwrite memory.

Is what I am doing undefined behavior? Is there a way to fix this?

EDIT: With the help of the answer below, I have re-written the code. In case anyone wants to see the implementation, below is the revised code

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

typedef struct {
    int pub;
} class;

typedef struct {
    class public;
    int PV;
} classSource;

int class_getPV(class *c) {
    return ((classSource*)c)->PV;
}

void class_setPV(class *c, int newPV) {
    ((classSource*)c)->PV = newPV;
}

class *class_init() {
    classSource *cs = malloc(sizeof(*cs));
    if((void*)cs == (void*)NULL) {
        printf("Error: malloc failed to allocate memory");
        exit(1);
    }
    cs->public.pub = 10;
    cs->PV = 8;
    return &(cs->public);
}

int main() {
    class *c = class_init();
    class_setPV(c,4524);
    printf("%d\n",class_getPV(c));
    printf("%d\n",c->pub);

    free(c);
    return 0;
}

Answer 1

There are at least three separate problems in your code.

1. You don't actually have a "struct otherwise the same but without the hidden properties". Your class and classSource structs have their getPV and setPV members in different places. Internally member access boils down to byte offsets from the beginning of the struct. To have a fighting chance of working, your code would need to have a common initial prefix of members between the two struct types (i.e. get rid of int apv; or move it to the end).

2. You're returning a struct by value, which automatically makes a copy. You've reimplemented the object slicing problem: Because the return value has type class, only the members of class will be copied. The extra members of classSource have been "sliced off".

3. You're using nested functions. This is not a standard feature of C; GCC implements it as an extension and says:

   If you try to call the nested function through its address after the containing function exits, all hell breaks loose.

   This is exactly what's happening in your code: You're calling c.setPV(3452); and c.getPV after class_init has returned.

If you want to fix these problems, you'd have to:

1. Fix your struct definitions. At minimum all members of class need to appear at the beginning of classSource in the same order. Even if you do that, I'm not sure you wouldn't still run into undefined behavior (e.g. you might be violating an aliasing rule).

   I'm somewhat sure that embedding one struct in the other would be OK, however:

   typedef struct classSource {
       class public;
       int PV;
   } classSource;
   

   Now you can return &cs->public from your initializer, and your methods can cast the class * pointer back to classSource *. (I think this is OK because all struct pointers have the same size/representation, and X.public as the first member is guaranteed to have the same memory address as X.)

2. Change your code to use pointers instead. Returning a pointer to a struct avoids the slicing problem, but now you have to take care of memory management (malloc the struct and take care to free it later).

3. Don't use nested functions. Instead pass a pointer to the object to each method:

   class *c = class_init();
   c->setPV(c, 3452);
   int x = c->getPV(c);
   

   This is somewhat tedious, but this is what e.g. C++ does under the hood, essentially. Except C++ doesn't put function pointers in the objects themselves; there's no reason to when you can either use normal functions:

   setPV(c, 3452);
   int x = getPV(c);
   

   ... or use a separate (global, constant, singleton) struct that just stores pointers to methods (and no data). Each object then only contains a pointer to this struct of methods (this is known as a vtable):

   struct classInterface {
       void (*setPV)(class *, int);
       int (*getPV)(const class *);
   };
   static const classInterface classSourceVtable = {
       class_setPV,  // these are normal functions, defined elsewhere
       class_getPV
   };
   

   Method calls would look like this:

   c->vtable->setPV(c, 1234);
   int x = c->vtable->getPV(c);
   

   But this is mainly useful if you have several different struct types that share a common public interface (class) and you want to write code that works uniformly on all of them.