0

I am trying to send and receive a following struct via send() and recv(). Following are the struct that I am trying to send/recv, my client.c, and server.c codes. It seems like my server is unable to deserialize the struct received. When I run the following server code, I am getting segfaults and the print statements after recv() aren't printing anything. Any advice on how to properly send and receive a struct?

typedef struct {
    char* path;
    int flags;
} OpenStruct;

client.c

OpenStruct *os = malloc(sizeof(OpenStruct));
char* pathname = "overflow.txt"
os -> path = pathname;
os -> flags = 1; 
int ret = send(socket_fd, &os, sizeof(OpenStruct), 0);

server.c

int openSize = sizeof(OpenStruct);
OpenStruct *os = (OpenStruct*)malloc(openSize);
char* buf1 = malloc(openSize); 
while ((rv = recv(sessfd, (void *)buf1, openSize, 0))>0) {
       memcpy(&os, buf1, intBuf);
       char* path = os -> pathname;
       fprintf(stderr, "content of path %s\n", path);
}
yunle
  • 11
  • 3
  • 1
    You doesn't seem serializing. Generally saving/sending pointers will be useless. – MikeCAT Jul 19 '22 at 11:05
  • Instead of sending the pointer, you should send the length of string, then send the string. Another option is rely on terminating null-character and not sending the length (send the string with terminating null-character only). – MikeCAT Jul 19 '22 at 11:07
  • A pointer is only valid in the process it was created in. No other process, not even one running the same program, will have the same memory map. You can't transfer pointers. Instead you need to come up with a way to transfer the *contents* of the memory that the pointer is pointing to, together with the size of the contents. Or find a library which can do it for you. – Some programmer dude Jul 19 '22 at 11:08
  • @MikeCAT string is just a part of my struct, and I would like to send a whole struct at once rather than sending each part of the struct separately. Do you have any suggestion for that? I can send a size of struct first maybe and send a struct? – yunle Jul 19 '22 at 11:11
  • @Someprogrammerdude would memcpy do the job? – yunle Jul 19 '22 at 11:11
  • 1
    How about not using pointers in the structure and instead putting `char` arrays with enough length on the structure? (it should be trade-off between extra space to send versus implementation cost) – MikeCAT Jul 19 '22 at 11:13
  • Copying the pointer would still just be copying the pointer, not the data it points to. – Some programmer dude Jul 19 '22 at 11:16
  • 1
    Also [in C you don't have to (and really shouldn't) cast the result of `malloc`](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/). – Some programmer dude Jul 19 '22 at 11:16
  • If one of the answers solved your problem you should accept that answer. That is how StackOverflow works. – Gerhardh Jul 27 '22 at 06:24

2 Answers2

1

error #1 - you fill your struct field with the pointer to the string not the data itself

os->path = pathname;

error #2 - os is already a pointer, you take the address of the pointer.

int ret = send(socket_fd, &os, sizeof(OpenStruct), 0);

The proper way would be something like this:

typedef struct {
    int pathSize;
    int flags;
} OpenStruct;

client.c:

char* pathname = "overflow.txt"
OpenStruct *os = malloc(sizeof(OpenStruct));
if (os == null) {
    perror("");
    return false;
}
os -> pathSize = htons(strlen(pathname));
os -> flags = 1; 

int ret = send(socket_fd, os, sizeof(OpenStruct), 0);
if (ret != sizeof(OpenStruct)) {
    perror("")
    return false;
}
ret = send(socket_fd, pathname, os->pathSize, 0);
if (ret != os->pathSize) {
    perror("")
    return false;
}

You get the idea

leo
  • 387
  • 1
  • 6
  • Any suggestion on how to resolve both errors? – yunle Jul 19 '22 at 11:09
  • The proper way should not really change the struct but only the way how to read and write it. Also taking padding and endianess into account would be a good idea. – Gerhardh Jul 19 '22 at 11:28
  • @Gerhardh There is an assumption that both client and server are on the same kind of machines and thus have the same endianess. Do you have any suggestion of keeping the struct but improving the way how I read and write it? – yunle Jul 19 '22 at 11:38
  • If you need to keep a path in a struct - you have to declare char[] field and memcpy() the path into it. So this field should be enough size to include path of any length. As for indianess - use `htons()` and `ntohs()` when sending and receiving the data – leo Jul 19 '22 at 11:41
  • @Gerhardh I disagree about not changing the struct. Struct's field path contains a pointer. This pointer will not have any sense on the other side of network connection. – leo Jul 19 '22 at 11:52
  • Yes, the pointer does not have any meaning. It is the job of serialization/deserialization to convert the struct's content to network representation and back. The result should still be the same. – Gerhardh Jul 19 '22 at 12:01
  • @Gerhardh, so your suggestion is to add a new layer of abstraction - serialization? It is good solution for a prod grade application but I believe it is out of the scope of this question :) – leo Jul 19 '22 at 12:04
  • 1
    Not an extra layer but an adapted function to read/write the struct. I believe it is exactly what the question is about. – Gerhardh Jul 19 '22 at 12:10
1

There should be tons of tutorials about serialization and deserialization.

The main purpose of that process is to convert your internal representation into some external representation that is reversible.

Normally this involves taking measures to deal with endianess and padding etc. But as you mentioned in comments, that should be ignored here.

A serialization function could look like this:

void serialize_OpenStruct(int socket_fd, OpenStruct *os)
{
  // TODO: Add check of sent_bytes.

  size_t len = strlen(os->pathname);
  send(socket_fd, &len, sizeof(len), 0);
  send(socket_fd, os->pasthname, len, 0);
  send(socket_fd, &os->flags, sizeof(os->flags), 0);
}

The corresponding deserialization would look like this:

OpenStruct *deserialize_OpenStruct(int socket_fd)
{
  // TODO: Add check of sent_bytes and malloced memory.

  OpenStruct *os = malloc(sizeof(OpenStruct));
  size_t len;

  recv(socket_fd, &len, sizeof(len), 0);
  os->pathname = malloc(len+1);
  recv(socket_fd, os->pasthname, len, 0);
  recv(socket_fd, &os->flags, sizeof(os->flags), 0);
  return os;
}

Alternatively you could only prepare some buffer instead of using sockets directly. That would allow to use the functions both for network communication and for storing the data in a file.

Gerhardh
  • 11,688
  • 4
  • 17
  • 39
  • 1
    I'd prefer this solution over mine. Can't suggest an edit, so send(... &os->flags ...) will not cause a segfault :) – leo Jul 19 '22 at 12:14