Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add version negotiation to network protocol #98

Merged
merged 4 commits into from
Aug 30, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/frontend.c
Original file line number Diff line number Diff line change
@@ -173,7 +173,7 @@ void parse_args(int argc, char *argv[], arguments_t *arguments) {
server = arg_strn("s", "server", "<SERVER>", 0, 1,
"address of server to connect to"),
port = arg_strn("p", "port", "<PORT>", 0, 1,
"port of server to connect to (default 5000)"),
"port of server to connect to (default 45011)"),
file = arg_filen(NULL, NULL, "[FILE]", 0, 1,
"filepath for read/write ('-' to read from stdin)"),
end = arg_end(20),
23 changes: 22 additions & 1 deletion src/network.c
Original file line number Diff line number Diff line change
@@ -19,7 +19,7 @@
fd_set testfds, clientfds;
char *msg_buf;
size_t msg_size;
int port = 5000;
int port = 45011;
int fd;
int sockfd;
FILE *sockstream;
@@ -29,6 +29,8 @@ struct hostent *hostinfo;
struct sockaddr_in address;
struct addrinfo hints, *servinfo;

const char *PROTOCOL_VERSION = "1.0";

/* Connects to server and returns its canvas
*
*/
@@ -61,6 +63,25 @@ Canvas *net_init(char *in_hostname, char *in_port) {

sockstream = fdopen(sockfd, "r+");

// "negotiate" protocol version
char version_request_msg[16];
snprintf(version_request_msg, 16, "v %s\n", PROTOCOL_VERSION);
if (write(sockfd, version_request_msg, strlen(version_request_msg)) < 0) {
perror("version negotiation: write error");
exit(1);
}

if (getline(&msg_buf, &msg_size, sockstream) == -1) {
perror("version negotiation: read error");
exit(1);
}
if (!(msg_buf[0] == 'v' && msg_buf[1] == 'o' && msg_buf[2] == 'k')) {
eprintf("Failed to negotiate protocol version: the server says '%s'\n",
msg_buf);
exit(1);
}

// receive canvas from server
getline(&msg_buf, &msg_size, sockstream);
char *command = strtok(msg_buf, " ");
if (!strcmp(command, "cs")) {
34 changes: 33 additions & 1 deletion src/server.c
Original file line number Diff line number Diff line change
@@ -25,6 +25,8 @@
static _Atomic unsigned int cli_count = 0;
static int uid = 10;

const char* PROTOCOL_VERSION = "1.0";

#define MAX_CLIENTS 100
#define BUFFER_SZ 2048

@@ -154,6 +156,35 @@ void *handle_client(void *arg) {
print_client_addr(cli->addr);
printf(" referenced by %d\n", cli->uid);

// protocol negotiation
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's not a bad use of GOTO, but I'm not convinced it's a good one either?
You could do the exact same thing with a "negotiate protocol" void function, and at a glance the function signature shouldn't even be too bad (especially if you pass rlen as a parameter).
Also if else statements could work, at the cost of some duplicate code (which I assume is why you got stoked about GOTO in the first place).
I vote for the function, but if you do want to keep the GOTO statement, watch your 6.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do the exact same thing with a "negotiate protocol" void function, and at a glance the function signature shouldn't even be too bad (especially if you pass rlen as a parameter).

Fair. I'm trying to hold off on refactoring any server code to avoid more merge conflicts with #80, which already restructures a fair bit of it.

I'll rewrite this bit as part of #80.

if ((rlen = read(cli->connfd, buff_in, sizeof(buff_in) - 1)) < 0) {
printf("version negotation: error reading from socket\n");
goto CLIENT_CLOSE;
}
buff_in[rlen] = '\0';
strip_newline(buff_in);
char* cmd = strtok(buff_in, " ");
if (cmd == NULL || cmd[0] != 'v') {
printf("version negotiation: client command not 'v'\n");

goto CLIENT_CLOSE;
}
char* client_version = strtok(NULL, " ");
if (client_version == NULL) {
printf("version negotiation: unable to parse client version\n");
send_message_self("can't parse version\n", cli->connfd);
goto CLIENT_CLOSE;
}
if (strcmp(client_version, PROTOCOL_VERSION) != 0) {
printf("version negotiation: unknown client protocol version: '%s'\n", client_version);
send_message_self("unknown protocol - supported protocol versions: ", cli->connfd);
send_message_self(PROTOCOL_VERSION, cli->connfd);
send_message_self("\n", cli->connfd);
goto CLIENT_CLOSE;
}
send_message_self("vok\n", cli->connfd);

// send canvas
sprintf(buff_out, "cs %d %d\n", canvas->num_rows, canvas->num_cols);
send_message_self(buff_out, cli->connfd);
printf("sent canvas size\n");
@@ -199,6 +230,7 @@ void *handle_client(void *arg) {
send_message_self(canvas_buf, cli->connfd);
}
}
CLIENT_CLOSE:

/* Close connection */
close(cli->connfd);
@@ -261,7 +293,7 @@ int main(int argc, char *argv[]) {
pthread_t tid;

/* Socket settings */
int port = 5000;
int port = 45011;
listenfd = socket(AF_INET, SOCK_STREAM, 0);
serv_addr.sin_family = AF_INET;
serv_addr.sin_addr.s_addr = htonl(INADDR_ANY);