Relearning C
20th February 2024
I read the HTTP spec. Well I read most of it. It’s not an exciting document and there are parts of it where my brain wandered off. I am fairly certain I didn't miss anything important. If you are struggling to sleep at night, you can check out RFC 9110 the international standard for HTTP for yourself.
I can’t on the surface see what I’m doing wrong, my understanding of HTTP was broadly correct. There are a few response headers marked with a SHOULD that I’m not currently including, notably Date, but these shouldn’t cause the problems I’m seeing. I added the Date header anyway to be sure it didn’t fix the problem, and it didn’t.
One thing I was doing incorrectly, was only using a new line feed (\n) as a line separator in my responses, this should be a carriage return and a line feed (\r\n). I actually corrected this several days ago and forgot to mention it, but it also didn’t fix the problem. As it happens the browsers seemed quite happy with just line feeds, I assume there is some tolerance built in but it is better that I follow the standards.
I was starting to think that the problem is not with how I’m formatting the response but how I’m handling multiple sockets and/or multiple requests. The problem with Chrome failing to complete a request only seems to occur when there are multiple resources to load, for example the HTML and an image. Maybe it is using that second socket it opens but I’m sending my response to the wrong place. Maybe this is why the request never seems to finish as the socket Chrome is waiting on never gets any data.
I’m not sure if it’s because I was re-using the same two buffers for reading the request and composing the response for all sockets. I assumed this would be ok because my program is single threaded so I’m dealing with each socket in turn. In my head I read the request, parse it, compose a response and send it in a single sequence, so the buffers are then available to be re-used for the next socket in the list. It’s possible I messed up the way I’m re-using the buffers. It's also possible that I’ve misunderstood how sockets work and while I’ve finished with the buffers, under the covers the socket has not. Or I could have just made an error in logic somewhere.
Whatever the reason, I decided I needed to take a step back, review the basics of how strings, arrays, pointers and memory works in C. Then review all the code up to this point with a better, more informed understanding of how everything should work.
Back to basics
Strings
I know how strings work in C, I’ve learnt it a few times, but when you don’t use it every day and spend most of your life working with languages that deal with strings in a different way, you forget the subtleties. Also when you go and build a dynamic list of strings and end up with a pointer to a pointer (to another pointer?) and you're not used to the syntax, your brain is at risk of melting. So I wrote a couple of test programs to mess around and remind myself how it all works.
I know a string is just an array of characters followed by a null terminator (aka a byte set to zero). This means you need to make sure you have enough space for the characters and one extra byte for the null terminator. It is particularly important because when you pass a string to a function, what you are actually doing is passing a pointer to the first character, that function will then keep stepping through memory until it finds that null terminator. Create an array of characters without that null terminator and pass it to a function that is expecting one and you have an overflow, the cause of many bugs and security exploits.
I wrote some code to see an overflow in the wild, run on your computer at your own risk.
char hello[5] = "hello"; // force a non-terminated array of characters
char world[] = "world";
printf("hello size:%d\n", sizeof(hello)); // 5
printf("world size:%d\n", sizeof(world)); // 6, the 5 characters and the null terminator
printf("hello strlen:%d\n", strlen(hello)); // undefined behaviour
// probably 10 as it overflows into world
printf("world strlen:%d\n", strlen(world)); // 5
printf("%s %s\n", hello, world); // undefined behaviour, probably "helloworld world"Alternatively, for some functions you need to tell the function how many characters (or bytes) to process. For example the send function takes a pointer to any buffer and the number of bytes to read from it. In reality the send function knows nothing and cares not about strings, it just sees bytes. Some functions do both, for example strnlen will look for the null terminator, but over a maximum of n bytes. Most of the str* functions have a strn* variant so you can make overflow safe code.
Returning Strings
Returning a string from a function is also a pain in the backside. It is a problem with memory management and how when returning a value from a function, the system is actually copying the value from one stack frame to another. This is not a problem for scaler variables that have a known size and are generally small. With strings of varying sizes, it becomes an issue.
Generally speaking there are two solutions. Don’t store the string on the stack and instead use the heap, the function can then return a pointer to the string on the heap. The problem here is the callee has to remember to free the memory when it’s done with the string, easily missed and/or forgotten. The second option is for the callee to create a buffer in its stack frame with enough space for the string and then pass a pointer to the buffer to the function. The problem here is you have to make an educated guess on how much space to allocate and code defensively or end up with a buffer overflow.
I will probably use a combination of the two approaches, picking the appropriate one depending on the circumstances. It’s not a big issue, just something I have to be careful of because after years of coding in JavaScript and LotusScript my default thinking is invalid. For example, I wanted to create a function that generated a time stamp as a string to use in my logging. I started with the following code to test my understanding of the time functions.
void time_stamp() {
time_t seconds = time(NULL);
struct tm* gmt = gmtime(&seconds);
printf("%02d:%02d:%02d\n", gmt->tm_hour, gmt->tm_min, gmt->tm_sec);
}This worked nicely. I then wanted to change this to return the value as string instead of printing it, so I updated the code as follows.
char* time_stamp() {
time_t seconds = time(NULL);
struct tm* gmt = gmtime(&seconds);
char rv[9];
sprintf(rv, "%02d:%02d:%02d", gmt->tm_hour, gmt->tm_min, gmt->tm_sec);
return rv;
}This resulted in a warning when compiling.
src/test.c: In function ‘time_stamp’:
src/test.c:17:16: warning: function returns address of local variable [-Wreturn-local-addr]
17 | return rv;
| ^~
I then realised what I had done. The rv (return value) is allocated in the function’s stack frame and will be unallocated when the function returns. So the pointer I’m returning will point to an invalid location before I get a chance to use it in the calling function. The compiler caught it and warned me. Thank you compiler. The correction is to pass a pointer to a buffer allocated higher up (or is that lower down) the stack.
#include <stdio.h>
#include <time.h>
char* time_stamp(char* buf, size_t buf_size) {
time_t seconds = time(NULL);
struct tm* gmt = gmtime(&seconds);
//snprintf(buf, buf_size, "%02d:%02d:%02d", gmt->tm_hour, gmt->tm_min, gmt->tm_sec);
strftime(buf, buf_size, "%H:%M:%S", gmt); // same idea as line above, but cleaner
return buf;
}
void main(void) {
char date_buf[9];
printf("%s\n", time_stamp(date_buf, sizeof(date_buf)));
}Higher level programming languages hide all this complexity away, returning a string from a function is a no brainer. Thinking about it for a second, I’m assuming they store strings on the heap, then use garbage collection to free up the memory once it’s no longer in use.
I’ve skipped over some detail here, just casually talking about the stack and the heap like everyone knows what they are. I’m not sure I'm getting the level of detail for these posts correct, I should probably tell people about the blog and get some feedback. That said, I have to remind myself this is a journal and not a tutorial. I can always return to these topics later if I decide to explore them in more detail. In the meantime if you have no idea what the stack or heap is but want to know more, I recommend a google search.
Lists
Next up I wanted to play with arrays that could grow. This code is just an evolution of what I’d already copied from Beej’s guide for maintaining an array of sockets for polling. The big change is I wrapped up the three variables I need to track the array (the current size, the current count and a pointer to the array) in a structure. This way I could pass a single thing to the various helper functions when adding and removing entries. This is essentially a poor man's object orientation.
#include <stdlib.h>;
#include <stdio.h>;
struct string_list {
int size;
int count;
char** things;
};
struct string_list* string_list_new() {
struct string_list* list;
if ((list = malloc(sizeof(*list))) == NULL) {
fprintf(stderr, "Unable to allocate memory for string list");
return NULL;
}
list->size = 5;
list->count = 0;
if ((list->things = malloc(sizeof(*list->things) * list->size)) == NULL) {
fprintf(stderr, "Unable to allocate memory for string list");
return NULL;
}
return list;
}
void string_list_free(struct string_list* list) {
free(list->things);
free(list);
}
int string_list_add(struct string_list* list, char* new_thing) {
if (list->count == list->size) {
int new_size = list->size * 2;
char** new_things = realloc(list->things, sizeof(*list->things) * new_size);
if (new_things == NULL) {
fprintf(stderr, "Unable to allocate memory for string list");
return -1;
}
list->size = new_size;
list->things = new_things;
}
list->things[list->count] = new_thing;
list->count++;
return list->count-1;
}
void string_list_rm(struct string_list* list, int index) {
if (index>=0 && index<list->count) {
list->things[index] = list->things[list->count-1];
list->count--;
}
}
void string_list_dump(struct string_list* list) {
printf("-----------------\n");
printf("string list: %d/%d\n", list->count, list->size);
printf("-----------------\n");
for(int i=0; i<list->count; i++) {
printf("%d: %s\n", i, list->things[i]);
}
}
int main(void) {
// test code
struct string_list* strings = string_list_new();
string_list_add(strings, "hello");
string_list_add(strings, "world");
string_list_add(strings, "everyone!");
string_list_rm(strings, 1);
string_list_dump(strings);
return 0;
}Linked Lists
I then played around with structures a bit more and made myself a linked list of strings. To stretch my pointer logic muscles, I used a pointer to a pointer to keep track of the trailing next pointer. This would speed up add operations because I don’t have to search the list for the end. It may have been conceptually easier to track the tail node but then the code would have been more complex as I have to deal with more edge cases like when the list is empty. Tracking the pointer and not node means I can treat the head pointer and the next pointer in any node the same. And it helped boost my confidence with multilevel pointers.
#include <stdlib.h>
#include <stdio.h>
struct node {
struct node* next;
char* data;
};
struct linked_list {
struct node* head;
struct node** tail_next;
};
struct linked_list* linked_list_new() {
struct linked_list* list;
if ((list = malloc(sizeof(*list))) != NULL) {
list->head = NULL;
list->tail_next = &list->head;
}
return list;
}
void linked_list_free(struct linked_list* list) {
struct node* node = list->head;
while (node != NULL) {
struct node* next_node = node->next;
free(node);
node = next_node;
}
free(list);
}
struct node* linked_list_add(struct linked_list* list, char* new_data) {
struct node* new_node;
if ((new_node = malloc(sizeof(*new_node))) != NULL) {
new_node->next = NULL;
new_node->data = new_data;
*list->tail_next = new_node;
list->tail_next = &new_node->next;
}
return new_node;
}
void linked_list_rm(struct linked_list* list, struct node* rm_node) {
struct node** prev = &list->head;
for(struct node* node = list->head; node!=NULL; node = node->next) {
if (node == rm_node) {
*prev = node->next;
if (list->tail_next == &node->next) {
list->tail_next = prev;
}
free(node);
return;
}
prev = &node->next;
}
}
void linked_list_dump(struct linked_list* list) {
printf("-----------------\n");
printf("linked list\n");
printf("-----------------\n");
struct node* node = list->head;
while (node != NULL) {
printf("%s\n", node->data);
node = node->next;
}
}
int main(void) {
// test code
struct linked_list* ll = linked_list_new();
linked_list_add(ll, "one");
struct node* two = linked_list_add(ll, "two");
linked_list_add(ll, "three");
linked_list_rm(ll, two);
linked_list_dump(ll);
return 0;
}Callback functions
I also reminded myself how to define a function that takes a pointer to a function as an argument so it can be called later. The syntax can get a bit ugly defining the function pointer type, but this can be mitigated using a typedef to give it a nice name. Also because C is weird, you don’t have to dereference a function pointer to use it. In-fact the dereference operator just returns the same pointer to the function. So does the address-of operator. This is somewhat different from how it works with normal variables but makes the syntax for using a function pointer very clean.
#include <stdio.h>
int add(int a, int b) {
return a + b;
}
int mul(int a, int b) {
return a * b;
}
int do_sum(int (*callback)(int, int), int a, int b) {
return callback(a, b);
}
typedef int (*op_t)(int, int);
int also_do_sum(op_t callback, int a, int b) {
return callback(a, b);
}
int main(void) {
printf("direct\n");
printf("2+3=%d\n", add(2,3));
printf("2*3=%d\n", mul(2,3));
printf("callback\n");
printf("2+3=%d\n", do_sum(add,2,3));
printf("2*3=%d\n", do_sum(mul,2,3));
printf("typedef callback\n");
printf("2+3=%d\n", also_do_sum(add,2,3));
printf("2*3=%d\n", also_do_sum(mul,2,3));
return 0;
}Clean Tinn
Feeling much more comfortable with C, it was time to implement my lessons learnt and start the big refactor of all my code up to this point.
Static files
I started with how I was storing the list of available files the server could serve. I created a linked list of strings. In fact two strings, one being the local path, the other being the server path. Maybe overkill but it replaces the statically sized 2D array I was using before. That was a ticking time bomb waiting to crash my program. I also have vague intentions to expand the list to store other useful information at a later date, like the date the file was last modified, maybe even an in-memory cache of the file.
I also changed the program so it doesn't just serve the current directory (and its descendants). Now you have to pass a command line argument to the program to specify the directory. Much more convenient.
Sockets list
I created a structure to manage the array of open sockets. This started very similar to the string list example above, which as I said was based on the code I already had to manage the array of open sockets.
However, for every socket I needed more than just the poll file descriptor. I wanted to move the code that processes the socket whenever it has data to a separate function. I also wanted a flexible way to store state information about each socket. To achieve this I created two additional arrays, one of function pointers and one of magical void pointers (pointers that can point to anything).
All three arrays should always contain the same number of elements, I can therefore re-use the size and count variables used to manage them. I can also use the same add and remove functions to update the arrays and keep them aligned.
Main loop
I could now move the code for handling sockets out of the main loop and out of the main function. I created two functions: server_listener and client_listener to handle each type of socket and passed these to the socket list. This made the code in the main network loop very short and tidy.
// loop forever directing network traffic
for (;;) {
if (poll(sockets->pollfds, sockets->count, -1) < 0 ) {
perror("poll");
return EXIT_FAILURE;
}
for (int i = 0; i < sockets->count; i++) {
if (sockets->pollfds[i].revents & POLLIN) {
sockets->listeners[i](sockets, i);
}
}
}Server listener
The server_listener’s main job is to accept client connections and add the new client socket to the list of sockets. In addition to the socket, this code also creates a pointer to a client_state structure which I use to store state information about each client including the client’s IP address and an input buffer for requests.
Client listener
Up to this point I was quite pleased with my refactor efforts. I’d broken the code down into nicely sized functions with everything looking quite neat. Then I wrote the client_listener function and it’s a mess. Part of the problem is this is where the actual logic of a web server needs to go and part of it is I’m not ready to write a full on interpreter yet to properly parse the incoming request. That job is moving swiftly up my priority list but for now I just hacked it together.
The code reads as much data as it can. It then loops over each character looking for the location of the first space character, and the second space character, and a blank line. If it can’t find a blank line, the request header is not complete and so the code doubles the size of the input buffer and will try to read the rest of the request on the next go around the main poll loop.
If it found a blank line, the request (header) has been received, so it can be processed. This is where the location of the first two spaces are important. They define the first two words in the request aka the method and the path. It’s then a matter of routing the request and returning the correct content if it’s a valid request or an error code if not.
Compared to the original code I think the important difference is each socket has its own buffer for the incoming request (stored in the client_state structure) and I read all the way to the blank line before processing the request and attempting to send a response. Considering that if the buffer fills up it can take a few times round the main poll loop to read the entire request, it seems silly I tried to do this before with a single input buffer. To be fair to me, I was trying to just read the first two words and discard the rest of the request. It would seem that it doesn’t work that way. I could be wrong and this made no difference, but (spoiler warning) my new version does seem to be fixed.
Reflecting on what I was seeing in the network tab in the Chrome developer tools, the original version would stall at the initialising connection phase, not in downloading the response. It would seem then that the problem did lie with how the server was reading the request and not how it was formatting and sending the response. Since then I did (accidentally) create a malformed response with truncated content and the status of the request in the Chrome developer tools tab was clear the stall was in the download phase. The tools were pointing me in the right direction, I just didn’t know how to interpret them correctly. A lesson learn’t.
Buffers
The last big change I made was creating a series of functions to handle expandable buffers. I probably over engineered this but it helped me further my understanding of building character arrays incrementally. I’ve used them for receiving the request from the clients and for composing the responses.
As I reflect on this code, there is a quiet voice in the back of my mind telling me I need to read up on streams in C. Something to add to the reading list.
Does it work?
Yes it does. I’m not getting stalled sessions anymore and the server seems much more responsive in general.
I’m fairly certain giving each client socket its own input buffer and making sure I read the entire request before responding is what fixed my problems. Equally it could be my improved understanding of arrays and strings in general and I’ve corrected a weird memory bug. Whatever the reason, I’m much happier with the code now and I’m beginning to feel more comfortable that I know what I’m doing.
I’ve tagged the current code as v0.3.0 on github.
What’s next
Testing, more testing, always more testing. I’m going to add some CSS files to the mix and see what that breaks. But the next big priority is to get the server to properly parse and act upon the incoming request headers. Which means I really should read that book I have on interpreters.
TC