Simple echo server

echod.c

#include <stdlib.h>
#include <netinet/in.h>
#include <string.h>
#include <stdio.h>
#include <unistd.h>

#define MAX_CONNECTIONS 5
#define BUFSIZE 1024

typedef struct {
  int fds[MAX_CONNECTIONS];
  int num;
} connections_t;

void die(char *msg) {
  perror(msg);
  exit(1);
}

unsigned short get_port(int argc, char **argv) {
  if (argc != 2) {
    fprintf(stderr, "usage: %s <port>\n", argv[0]);
    exit(1);
  }

  return (unsigned short) atoi(argv[1]);
}

void bind_address(int listener, unsigned short port) {
  struct sockaddr_in address;
  memset(&address, 0, sizeof(address));
  address.sin_family = AF_INET;
  address.sin_addr.s_addr = htonl(INADDR_ANY);
  address.sin_port = htons(port);

  if(bind(listener, (struct sockaddr *) &address, sizeof(address)) < 0) {
    die("ERROR on binding");
  }
}

int create_listener(unsigned short port) {
  int listener = socket(AF_INET, SOCK_STREAM, 0);
  if (listener < 0) { die("ERROR opening socket"); }

  bind_address(listener, port);

  if(listen(listener, 5) < 0) { die("ERROR on listen"); }

  return listener;
}

int set_fds_for_connections(connections_t *connections, fd_set *fds, int fd_max) {
  int curr_fd, i;
  for(i = 0; i < (*connections).num; i++) {
    curr_fd = (*connections).fds[i];
    FD_SET(curr_fd, fds);
    if(curr_fd > fd_max) { fd_max = curr_fd; }
  }

  return fd_max;
}

int set_fds_for_select(int listener, connections_t *connections, fd_set *fds) {
  int fd_max = listener;

  FD_ZERO(fds);
  FD_SET(listener, fds);
  return set_fds_for_connections(connections, fds, fd_max);
}

void add_connection(connections_t *connections, int fd) {
  if((*connections).num + 1 > MAX_CONNECTIONS) { die("ERROR too many connections"); }
  (*connections).fds[(*connections).num++] = fd;
}

int accept_connection(int listener) {
  struct sockaddr_in address;
  socklen_t address_size = sizeof(address);
  int fd = accept(listener, (struct sockaddr *) &address, &address_size);
  if(fd < 0) { die("ERROR on accept"); }

  return fd;
}

void handle_connection(int fd) {
  char buf[BUFSIZE];
  memset(&buf, 0, BUFSIZE);

  int n = read(fd, buf, BUFSIZE);
  if(n < 0) { die("ERROR reading from socket"); }

  printf("server received %d bytes: %s", n, buf);

  n = write(fd, buf, strlen(buf));
  if(n < 0) { die("ERROR writing to socket"); }
}

void handle_echo(connections_t *connections, fd_set *fds) {
  int curr_fd, i;
  for(i = 0; i < (*connections).num; i++) {
    curr_fd = (*connections).fds[i];
    if(FD_ISSET(curr_fd, fds)) {
      handle_connection(curr_fd);
    }
  }
}

void handle_connections(int listener, connections_t *connections) {
  fd_set fds;
  int fd_max = set_fds_for_select(listener, connections, &fds);
  int ready = select(fd_max + 1, &fds, NULL, NULL, NULL);

  if(ready < 0) { die("ERROR on select"); }
  if(ready == 0) { return; }

  if(FD_ISSET(listener, &fds)) {
    add_connection(connections, accept_connection(listener));
  }

  handle_echo(connections, &fds);
}

int main(int argc, char **argv) {
  unsigned short port = get_port(argc, argv);

  int listener = create_listener(port);

  connections_t connections;
  memset(&connections, 0, sizeof(connections));

  while(1) {
    handle_connections(listener, &connections);
  }
}

Doesn't actually work yet...but it compiles!

Ok fixed the main problem, but now there's something wonky with the buffer when you make more than one connection.

Doy. Clear the buffer. Now it works.

In set_fds_for_connections, why pass connectionsas a pointer? You're not changing it. Also, if you must, at least say connections->fds[i].

Other than that, looks good!

It's overkill for this, but Gnu getopt is a nice library for parsing command line arguments.

My thought on passing it as a pointer was to avoid copying. I'm not incredibly savvy with regards to the semantics of passing non-trivially-sized structs by value and/or the compiler's. Now, I suppose my struct happens to be trivially sized, but that was an arbitrary choice.

I went back and forth on the arrow syntax...I don't write enough C (obviously!) to have developed a strong sense of style, but I'm not convinced that the arrow syntax reads better to me. I feel like I can read the dereference and access left-to-right - "ok first I dereference the pointer, now I access the element" - whereas I have to back-track a tiny bit for the arrow syntax - "ok I'm using a pointer to this, oh, it's an arrow, so now I need to think of it as dereferenced, ok, now access the element". Pretty weak argument :)

It's more a case of being idiomatic. Generally, when presented with a pointer (which is a fine decision), experienced C programmers will almost always dereference it with -> instead of (*). But that's really a nitpick.