Refactoring C CodeChoosing the next direction

time to read 3 min | 443 words

Initially I thought that my next step would be to write the code to handle more than a single connection at a time, but I decided that this isn’t the natural next step. It would be better to build the API and abstractions that are required to use this network protocol before committing to an actual implementation. I took a peek at what is available, and it looks like we have the following options:

  • Thread per connection – obviously out.
  • select() / pool() – allow to have a single thread manage multiple connections, but require a lot of state management.
  • libuv – or similar, that handle all of those details for me.

Regardless of what I’ll end up in the end, I think that the externally facing API is always going to be the same, so I’m going to focus on that. Right now we have:

image

In other words, we have an interface that explicitly require us to manage state, manage the socket, etc. I think the first thing to do would be to change that. The first thing I did was to remove the connection_create and connection_read. They are now handled internally and no longer the concern of the calling code. Instead, we have:

image

This is basic OO in C, we define a bunch of function pointers and allow the user to send them to us to be trigger appropriately. On connection create, we’ll call connection_create(), and the user can return a state object that will be used in future callbacks. I’m currently using connection_recv() that accepts a raw buffer, because I haven’t dealt with parsing yet.

Note the call to server_state_run(), which actually does all the work. I’m currently just going to take whatever we already have and convert to this API.

That turned out to be fairly straightforward to do, I’m happy to say. I did find that working on C code after midnight has negative implications, I wrote:

image

Do you see the bug? The first time I’m adding a new cert, I’m going to allocate a single byte, then I’m going to write the full certificate_thumbprint to that memory, well beyond the buffer I actually allocated. The fix?

image

That’s simple enough, but it was an unpleasant thing to realize that I made this error.

More posts in "Refactoring C Code" series:

  1. (10 Dec 2018) Do we need a security review?
  2. (07 Dec 2018) Implementing parsing
  3. (04 Dec 2018) Multi platform and Valgrind
  4. (03 Dec 2018) Choosing the next direction
  5. (30 Nov 2018) Giving good SSL errors to your client…
  6. (29 Nov 2018) Starting with an API
  7. (27 Nov 2018) Error handling is HARD, error REPORTING is much harder