You can never be too sure

Programmers can be compulsively paranoid. Yesterday I encountered some code that was very careful about closing a file once it was done with it:

FILE *fp = NULL;
while (a loop which is worth a post in its own right) {
  fp = fopen(somefile, "r");
  if (fp) {
    //Read from fp...
    fclose(fp);
    fp = NULL;
  }
  if (fp) {
    fclose(fp);
    fp = NULL;
  }
}
if (fp) {
  fclose(fp);
  fp = NULL;
}

There were no breaks or anything else that might evade the first fclose. But the extra fcloses were separated by a page or two of other cr^Hode, so it wasn't entirely obvious that they weren't necessary. And this was in a long-running process, so it wouldn't do to leak file descriptors. The author was just being careful! Yeah, that's it.

If closing a file requires that much care, what about opening one? A lot of programs just charge reckessly ahead and read from a file once they've opened it. Not this one. Instead it did this:

FILE *fp = fopen(filename, "r");
if (fp) {
  //But what if the file doesn't exist?
  stat stats;
  if (!stat(filename, &stats)) {
    //OK, read from fp...
  } else
    fprintf(stderr, "Error: stat failed for %s", filename);
  fclose(fp);
}

Yes, it used stat to verify the existence of a file it had just opened. I have no idea what the author thought this was supposed to accomplish.

It should come as no surprise that despite this excruciating attention to imaginary problems, this code was infested with real ones.