Another Windows 7 BSOD

Not quite 6 weeks since last time. Not doing anything except browsing the web.

I'm mostly okay with Windows 7, but I sure wish it felt more stable than the old Windows 95/98 days.

Update: on further review, this is a hardware problem, no reason to blame Windows 7 (well, at least for my crashes).

Just a couple notes for web admins

I already have an app for your website, it's called Firefox. Also, if you can't be bothered to make your website work in a mobile browser, what is there to make me think it will be better in your app?

If I can't use your website with my ad blocker, I won't use your website. The reality is, there are very few websites in my world that I can't live without. Bank, work, library, that's pretty much it. If your site puts up one of those "please turn off your ad blocker to proceed" blocks, I'm just leaving and not coming back. I get that you guys need to make money, but the Internet is a hideous place without ad blocker, and I'm not giving it up.

kilo: I've started my own version of kilo

Based on the comments I've made so far, I've started my own version of kilo, which you can find here if you are interested.

I've chosen to make some different design and stylistic choices from the previous versions. Most notably, I just can't restrict myself to a single source file. Even for a relatively small project like this, it's just too claustrophobic. I've also chosen the Unix style of underscores in identifiers (editor_process_keypress) rather than camel case (editorProcessKeypress), which feels anachronistic to me in the C/Unix environment.

I've also created a simple Perl script to help deal with the string constant issue I've mentioned previously. This allows arcane terminal escape sequences to be named symbolically, with their length computed at compile time. I expect a decent C optimizer will generate very similar code to the original inline versions.

I've also renamed struct abuf to the typedef-ed term_buffer, so the erstwhile editorRefreshScreen looks like this in my implementation:

void editor_refresh_screen()
{
    term_buffer tb;
    tb_init(&tb);
    
    string_const home_cursor = get_home_cursor_str();

    tb_append_str(&tb, get_cursor_off_str());
    tb_append_str(&tb, home_cursor);

    editor_draw_rows(&tb);

    tb_append_str(&tb, home_cursor);
    tb_append_str(&tb, get_cursor_on_str());
    
    tb_write(&tb);
    tb_free(&tb);
}

The functions get_home_cursor_str(), get_cursor_off_str(), and so on are automatically generated by my Perl script linked above. Notice how much nicer this is than the version shown in step 40:

void editorRefreshScreen() {
  struct abuf ab = ABUF_INIT;

  abAppend(&ab, "\x1b[?25l", 6);
  abAppend(&ab, "\x1b[2J", 4);
  abAppend(&ab, "\x1b[H", 3);

  editorDrawRows(&ab);

  abAppend(&ab, "\x1b[H", 3);
  abAppend(&ab, "\x1b[?25h", 6);

  write(STDOUT_FILENO, ab.b, ab.len);
  abFree(&ab);
}

kilo commentary: Wecome message (steps 41 and 42)

Steps 41 and 42 add a welcome message to our nascent editor implementation. It starts with another #define:

#define KILO_VERSION "0.0.1"

As I mentioned before, I'm not a fan of using the preprocessor when it can be avoided, and there's no reason not to declare this as a global character constant. But this is just awful:

void editorDrawRows(struct abuf *ab) {
  int y;
  for (y = 0; y < E.screenrows; y++) {
    if (y == E.screenrows / 3) {
      char welcome[80];
      int welcomelen = snprintf(welcome, sizeof(welcome),
        "Kilo editor -- version %s", KILO_VERSION);
      if (welcomelen > E.screencols) welcomelen = E.screencols;
      int padding = (E.screencols - welcomelen) / 2;
      if (padding) {
        abAppend(ab, "~", 1);
        padding--;
      }
      while (padding--) abAppend(ab, " ", 1);
      abAppend(ab, welcome, welcomelen);
    } else {
      abAppend(ab, "~", 1);
    }
    abAppend(ab, "\x1b[K", 3);
    if (y < E.screenrows - 1) {
      abAppend(ab, "\r\n", 2);
    }
  }
}

In a function called editorDrawRows fully half of the implementation is taken up with displaying the welcome message. We also see again the "reversed if" control flow organization:

if (condition)
  rare case
else
  usual case

I assume we'll see a lot of changes to this function as we go, but for now the implementation is greatly improved by reversing the ordering of the if statement, and moving the code to write the welcome message to its own function.

void editorDrawRows(struct abuf *ab) {
  int y;
  for (y = 0; y < E.screenrows; y++) {
    if (y != E.screenrows / 3) {
      abAppend(ab, "~", 1);
    } else {
      addWelcomeMessage(ab);
    }

    abAppend(ab, "\x1b[K", 3); /* erase to end of line */
    if (y < E.screenrows - 1) {
      abAppend(ab, "\r\n", 2);
    }
  }
}

Windows 7 just crashed on me

Total BSOD, sent to reboot land. Not the first time this has happened, either. I don't remember XP ever crashing on me like this.

This is not the 21st century I was promised.

Update: as noted above, this turns out to be a hardware problem.

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.

kilo commentary: getCursorPosition (step 33)

In step 33, this version of the getCursorPosition function is presented:

int getCursorPosition(int *rows, int *cols) {
  char buf[32];
  unsigned int i = 0;
  if (write(STDOUT_FILENO, "\x1b[6n", 4) != 4) return -1;
  while (i < sizeof(buf) - 1) {
    if (read(STDIN_FILENO, &buf[i], 1) != 1) break;
    if (buf[i] == 'R') break;
    i++;
  }
  buf[i] = '\0';
  if (buf[0] != '\x1b' || buf[1] != '[') return -1;
  if (sscanf(&buf[2], "%d;%d", rows, cols) != 2) return -1;
  return 0;
}

This is more of a minor stylistic issue compared to the previous stuff I've commented on, but I prefer to write loops like this using for:

int getCursorPosition(int *rows, int *cols) {
  if (write(STDOUT_FILENO, "\x1b[6n", 4) != 4) return -1;

  unsigned int i = 0;
  char buf[32];
  for (i = 0; i < sizeof(buf) - 1; i++) {
    if (read(STDIN_FILENO, &buf[i], 1) != 1) break;
    if (buf[i] == 'R') break;
  }
  buf[i] = '\0';

  if (buf[0] != '\x1b' || buf[1] != '[') return -1;
  if (sscanf(&buf[2], "%d;%d", rows, cols) != 2) return -1;
  return 0;
}

In this version, the initialization of i = 0 is contained within the loop, and the loop structure (IMO, anyway) is clearer. Whenever a loop is iterating on an integer variable with a hard upper bound, I just feel like a for loop makes it clearer what's happening. Also, since this project explicitly uses C99, there's no reason declare variables until they're actually needed.