Saturday, April 8, 2017

kilo commentary: editorDrawRows (step 29)

In step 25 and step 29 of implementing the kilo editor, a preliminary implementation of the function editorDrawRows is introducted:

void editorDrawRows() {
  int y;
  for (y = 0; y < E.screenrows; y++) {
    write(STDOUT_FILENO, "~\r\n", 3);
  }
}

This is meant to display a column of tildes on the left side of the screen, one on each row. But there's a bug, do you see what it is?

After writing out the final tide on the bottom most line of the terminal, it also outputs \r\n, carriage return line feed sequence, scrolling the screen. Of course, this is a major no-no for apps that want to control the screen as an editor like this does. It's not a huge deal in this very preliminary version of the code, and I assume it will be fixed later as we go, but it's something that bugged me when I noticed it.

The other (IMO more serious) problem with this (and other code in the kilo implementation) is the use of magic strings ("~\r\n") and numbers (3). This isn't that big of a deal in this tiny case, but as we'll see in my next post, the string constants aren't going to stay 3 characters long. A partial fix would be to introduce a simple wrapper function around write:

static inline ssize_t terminalWrite(const char* string)
{
  size_t sl = strlen(string);
  if ((size_t)write(STDOUT_FILENO, string, sl) == sl)
    return sl;
  return -1;
}

This will add the overhead of calling strlen every time a string is written, but particularly in an interactive environment, this should be a very minor issue. The magic string problem could be fixed by creating a list of global constants:

const char* g_tilde_rn = "~\r\n";
const char* g_tilde = "~";

We can then rewrite the function as follows:

void editorDrawRows() {
  int y;
  for (y = 0; y < E.screenrows; y++) {
    terminalWrite(y+1 < E.screenrows ? g_tilde_rn : g_tilde);
  }
}

Update: the extra \r\n bug is addressed in Step 35. This is a classic example of the fencepost version of the off-by-one type of bug.

No comments:

Post a Comment