Hiding in Plain Sight

Sometimes, the hardest bugs to find are the ones that hide in plain sight. They’re the sort of bug where your eyes skim over the offending code and your brain continuously says “yup, fine, right, good, yup” and you can’t spot the problem.

The other night I was working on some “fun code”, and ran into one of these bugs. While I’m happy to admit it didn’t cause me days of anguish, I still figured it was a good example to share with others as a simple warning: when debugging code, you must employ critical thinking skills!

I was working on reading in data from a particular file format, and the code that was failing for me was reading data from the target file using the fread API. The part of the file I was trying to read contained two-byte, ascending integer values starting from 0x0000, going to 0x01d0. I was attempting to read the values into an array of unsigned shorts using something like this:

// Error checking elided
::fseek( mFile, offset, SEEK_SET );
unsigned short *buffer = (unsigned short *)::calloc( sizeof( unsigned short ), numberOfEntries );
::fread( buffer, sizeof( unsigned short ), numberOfEntries, mFile );

What was strange to me was that the buffer didn’t seem to fill all the way. It would get to a certain point, and then “stopped” because the remainder of the buffer was filled with zeros. However, when I would look at the file on disk in a hex editor, I clearly saw all of the values in order.

At this point, I knew I was doing something wrong, but the call to fread was “so simple” that I was left scratching my head. So I began looking at nearby code to see if perhaps something else was at fault, but nothing jumped out at me. But I did get a tingling sensation while debugging — the buffer always terminated at the same spot, and it was always after reading in the value 0x00ff.

This turned out to be the key clue — the bug was with fopen. However, the call to fopen looked sane enough to me:

mFile = ::fopen( filePath, "r" );

I wanted to open up a file at the given path, and I wanted to open it up for reading. But my eyes overlooked a key point to fopen. When you open up a file with fopen and a mode of “r” for read, the file is opened in text mode. I’m so used to the Win32 APIs (CreateFile and ReadFile) that don’t distinguish between text and binary modes, so I simply didn’t spot the issue without giving it more thought. To understand why this was the root cause of my problem, you have to understand how fread operates.

The way fread works, according to the C99 specification, is by repeatedly calling fgetc to fulfill the request. fgetc returns the chars it finds, except that when it reads 0xff in text mode, it returns it as the EOF indicator because that’s not a valid character code. However, this behavior is system-dependent. For instance, Ctrl+Z is also a possible EOF character for a text file (0x1a). Regardless, the call to fgetc returns EOF, so fread has found the end of the file, and it stops reading the request.

The correct solution to the issue was to open the file in binary mode by using “rb” instead of “r” as the mode parameter. This caused fgetc to not interpret 0xff as the end-of-file indicator in the stream, so fread was able to read the entire block of data.

This is one of those bugs that’s quite obvious when you think about it, but is also very easy to miss when reviewing your code in the absence of further information. My eyes skimmed over the call to fopen as being “correct” several times before noticing the behavior in the debugger — after all, I was opening up the file in read mode and getting back a non-NULL pointer; it must be correct!

tl;dr: When it comes to debugging: think critically, question everything.

This entry was posted in C/C++ and tagged , , , . Bookmark the permalink.

Leave a Reply

Your email address will not be published.