Monday, November 06, 2006

.

Twenty Years of Schooling and They Put You on the Day Shift

Recycled
This is a re-post of an item I wrote for a company blog on 21 June 2005.

I recently found a bug in some ten-year-old C++ code, and I'd like to talk about it because it's a fine example of why good coding habits are a Very Good Idea. Here's the deal:

A former colleague wrote a C++ class called MimeDateTime, which included an attribute he called "timezone":

class MIME_EXPORT MimeDateTime {
      ...other stuff...
   private:
      struct tm tm;    // 'C' time structure
      int timezone;    // offset in minutes
                       // (local = UTC + timezone)
                       // if timezone=0 tm is UTC
                       // NOTE: Windows and OS/2 has the
                       // sign of their timezone
                       // the other way, we follow the
                       // international timezone
                       // standards with our sign
};

Now, Windows has a C++ global variable called "_timezone", which has the UTC offset in seconds (but with the sign reversed from what we needed), so in the implementation he put this, to set the value of the attribute:

// store the time zone offset from UTC in min
timezone = - _timezone/60;

Still with me? OK, but in Unix, the global variable is not called "_timezone", it's called "timezone", without the underscore. So he's got this at the beginning, to handle that situation:

#if defined(AIX) || defined(LINUX) /* Unix, no underscore */
   #define _timezone timezone
   #define _daylight daylight
#endif

If you're following this, you see where this has gone. As it turns out, this code hasn't mattered on Linux until now. But now it does, and we were getting bizarre time zones. Easy to track down, at least. Since the #define just causes text substitution, the code that set the value of the attribute "timezone" looked fine in the source, but amounted to this to the gcc compiler on Linux:

// store the time zone offset from UTC in min
timezone = - timezone/60;
...and the attribute "timezone" hid the global variable of the same name, which means that we were using the attribute's own uninitialized value to initialize it. So we got the garbage we deserved. For this reason, many of us use naming conventions to distinguish attributes from global variables from local variables from parameters. Changing the name of the attributes to "fTM" and "fTimeZone" corrected this problem and made the code easier to read, because we can tell at a glance that "fTimeZone" is an attribute, rather than having to go figure out what it is.

This is also a lesson in being very careful what you do with #define, because it can get you into trouble easily.

Or you can just program in java........

No comments: