Opaque pointer not accessable from resident .c file

Question

I am getting a weird segmentation fault when accessing a structure inside of a opaque structure form it's definition file. I have just recently learned about opaque pointers, but my my guess is, I am doing something wrong with the allocation of the internal structure. However, in a first call to the structure it seems to work and the it get unaccessible within the same method.

So I define my opaque structure pointer inside of connection.h:

#ifndef _Connection_h
#deifne _Connection_h
// Connection opaque pointer
typedef struct connection;
// API
_Bool connection_connect(struct connection *self);
#endif // _Connection_h

And then inside of connection.c I am allocating the internal serv structure pointer of type struct sockaddr_un.

#include "../connection.h"

struct connection {
  _Bool (*connect)();
  char *(*get_name)();
  char name[MAX_NAMELEN];
  char hostname[MAX_NAMELEN];
  uint serv_id;
  struct sockaddr_un *serv; // Server socket structure
};

// ** Public Interface **
_Bool connection_connect(struct connection *self) {
  printf("Generic connection (opaque pointer)..\n");
  strcpy(self->name, SERVERNAME);
  // Socket path
  char path[108];
  strcpy(path, SERVERNAME);
  strcat(path, "_socket");
  printf("self->name = %s\n", self->name);
  // Allocate the serv socket structure
  self->serv = malloc(sizeof(*self->serv));
  strcpy(self->serv->sun_path, path);
  self->serv->sun_family = AF_UNIX;
  printf("self->serv->sun_path = %s\n", self->serv->sun_path);
  printf("self->serv->sun_family = %hn\n", self->serv->sun_family);
  
  // Locate the host
  char hostname[MAX_NAMELEN];
  gethostname(hostname, MAX_NAMELEN);
  strcpy(self->hostname, hostname);

  if ((self->serv_id = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
    handle_error("Socket");
  return 1;
}

Then allocation of the connection interface methods is handled by client. For the minimal reproducible example a client type implementation may handle interface methods allocations like so:

#include "../connection.h"

typedef struct connection {
  void (*connect)();
} *client;

void client_connect(client self) {
  connection_connect(self);
  printf("Client connection..\n");
}

client make_client(client self) {
  client tmp = malloc(sizeof(client));
  tmp->connect = client_connect;
  return tmp;
}

// Main procedure
int main(int argc, char **argv) {
  client c = make_client(c);
  c->connect(c);
  return 0;
}

After execution the allocated structure first seem to be allocated correctly and it's instances are accessible until the last printf:

Generic connection (opaque pointer)..
self->name = freebay
self->serv->sun_path = freebay_socket
[1]    210938 segmentation fault (core dumped)

(gdb) p self->serv
$1 = (struct sockaddr_un *) 0x66203d2068746170
(gdb) x self->serv
0x66203d2068746170: Cannot access memory at address 0x66203d2068746170

My question is, why self->serv gets unaccessible after using it inside of the same method?

Answer 1

You're not allocating enough memory:

self->serv = malloc(sizeof(struct sockaddr_un *));

You're allocating enough space for a pointer to a struct sockaddr_un, not an instance of one. The pointer size is smaller than the struct size, so when you attempt to write to the struct you write past the end of allocated memory, triggering undefined behavior.

The proper way to allocate the space is:

self->serv = malloc(sizeof(struct sockaddr_un));

Or even better:

self->serv = malloc(sizeof(*self->serv));

As the latter doesn't care what the type is.

Also, this is incorrect:

printf("self->serv->sun_family = %hn\n", self->serv->sun_family);

Because %hn is expecting a short int *, not a short int. Change this to %hd.

---

With the full example you've given, we can now see that you have two incompatible structs with the same name in each of your .c files.

Your main file defines struct connection as:

typedef struct connection {
  void (*connect)();
} *client;

While connection.c defines it as:

struct connection {
  _Bool (*connect)();
  char *(*get_name)();
  char name[MAX_NAMELEN];
  char hostname[MAX_NAMELEN];
  uint serv_id;
  struct sockaddr_un *serv; // Server socket structure
};

Your main file creates an instance of the former in make_client with this:

client tmp = malloc(sizeof(client));

This is invalid by itself because again you're allocating space for a pointer and not an instance of the struct. It would need to be:

client tmp = malloc(sizeof(*tmp));

But even if you fixed that, you then pass this pointer to connection_connect which expects a pointer to the latter structure, not the former.

All functions that would create an instance of struct connection and modify its members should reside in connection.c where the real definition lives. That's how opaque structs are supposed to work.

Answer 2

Compiling your code I found this:

connection.c:37:43: warning: format specifies type 'short *' but the argument has type 'sa_family_t' (aka 'unsigned char') [-Wformat]
        printf("self->serv->sun_family = %hn\n", self->serv->sun_family);
                                         ~~~     ^~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

If you write:

printf("self->serv->sun_family = %cn\n", self->serv->sun_family);

Program has no error

Answer 3

You do not allocate self->serv->sun_path before copying data to it. Something must go wrong after that.

Except if sun_path is an array of char.