kilo commentary: Is this malloc Really Necessary?

Step 105 begins the process of saving the file to disk by mallocing a buffer the size of the entire file and then copying the file's text into it. Step 106 implements the code to write the contents of this buffer to a file.

There's really no good reason to do this. I suppose there's some speed gained by making a single call to write, rather than one for each line in the file. On the other hand, it seems unlikely to offset the cost of yet another dynamic allocation, and copying the entire contents of the file. A buffered writing system might be worth implementing, but this is not the way to do it.

kilo commentary: realloc is not magic

I'm now starting the "make this an actual text editor" part of implementing kilo. The first step, naturally, is to write code that inserts a new character. Here's the implementation of editorRowInsertChar given in step 101:

void editorRowInsertChar(erow *row, int at, int c) {
  if (at < 0 || at > row->size) at = row->size;
  row->chars = realloc(row->chars, row->size + 2);
  memmove(&row->chars[at + 1], &row->chars[at], row->size - at + 1);
  row->size++;
  row->chars[at] = c;
  editorUpdateRow(row);
}

So yeah, hence the title of this post. The call of memmove every time a character is inserted is pretty bad, too. But... well, this is actually probably good enough for a toy editor that no one is actually going to use for real editing work. Still, I wish the author had at least mentioned that this is very much a quick-and-dirty way to do it, and that there's better ways to do it.

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.