Monday, April 10, 2017

kilo commentary: Append Buffer (steps 36, 37, 38)

Starting with step 36, an "append buffer" implementation is presented, starting with these declarations:

struct abuf {
  char *b;
  int len;
};
#define ABUF_INIT {NULL, 0}

First of all, abuf is a exceedingly short and non-specific name to be putting in the global namespace. I know this is meant to be a relatively short program, but it's an old proverb in software development that big programs start their lives as small programs. I'm also not a huge fan of using the preprocessor to abstract away the initialization of a struct abuf value (I really don't like using the preprocessor at all when it can be avoided.) A compiler with a decent optimizer should generate comparable code for a call to something like this:

static inline void abInit(struct abuf* ab) {
  ab->b = NULL;
  ab->len = 0;
}

Another question I have about this implementation is, why use a dynamic buffer at all? As is shown in later steps, this is used to buffer output to the screen, instead of writing lots of short strings one at a time. It remains to be seen (by me, anyway) if this is its only use this data structure has, but it seems to me a relatively small fixed-size buffer would serve as well, without putting load on the dynamic allocation system:

struct abuf {
  char b[1024];
  int len;
};

I'd also point out that I'd really like to see clearing of the struct abuf fields in abFree:

void abFree(struct abuf& ab)
{
  free(ab->b);
  ab->b = NULL;
  ab->len = 0;
}

Leaving these with their old values is another bug waiting to happen. Finally, I'd add these utility abstraction functions to clarify the calling code:

static inline void abAppendStr(struct abuf* ab, const char* string) {
  abAppend(ab, string, strlen(string));
}

static inline void abWrite(struct sbuf* ab) {
  write(STDOUT_FILENO, ab->b, ab->len);
}

Note the use of static inline, so this adds a very helpful abstraction layer with zero cost at runtime.

No comments:

Post a Comment